Re: [RFC v2 0/5] Common Display Framework
On Fri, Feb 1, 2013 at 5:42 PM, Laurent Pinchart wrote: > Hi Rahul, > > On Wednesday 09 January 2013 13:53:30 Rahul Sharma wrote: >> Hi Laurent, >> >> CDF will also be helpful in supporting Panels with integrated audio >> (HDMI/DP) if we can add audio related control operations to >> display_entity_control_ops. Video controls will be called by crtc in DRM/V4L >> and audio controls from Alsa. > > I knew that would come up at some point :-) I agree with you that adding audio > support would be a very nice improvement, and I'm totally open to that, but I > will concentrate on video, at least to start with. The first reason is that > I'm not familiar enough with ALSA, and the second that there's only 24h per > day :-) > > Please feel free, of course, to submit a proposal for audio support. > >> Secondly, if I need to support get_modes operation in hdmi/dp panel, I need >> to implement edid parser inside the panel driver. It will be meaningful to >> add get_edid control operation for hdmi/dp. > > Even if EDID data is parsed in the panel driver, raw EDID will still need to > be exported, so a get_edid control operation (or something similar) is > definitely needed. There's no disagreement on this, I just haven't included > that operation yet because my test hardware is purely panel-based. one of (probably many) places that just keeping CDF (CDH? common display helpers..) inside DRM makes life easier :-P BR, -R > -- > Regards, > > Laurent Pinchart > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 0/4] Common Display Framework-TF
Hi Tomasz, Thank you for your RFC. On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote: > Hi, > > After pretty long time of trying to adapt Exynos-specific DSI display > support to Common Display Framework I'm ready to show some preliminary RFC > patches. This series shows some ideas for CDF that came to my mind during > my work, some changes based on comments received by Tomi's edition of CDF > and also preliminarys version of Exynos DSI (video source part only, still > with some FIXMEs) and Samsung S6E8AX0 DSI panel drivers. > > It is heavily based on Tomi's work which can be found here: > http://thread.gmane.org/gmane.comp.video.dri.devel/78227 > > The code might be a bit hacky in places, as I wanted to get it to a kind > of complete and working state first. However I hope that some of the ideas > might useful for further works. > > So, here comes the TF edition of Common Clock Framework. > > > Changes done to Tomi's edition of CDF: > > - Replaced set_operation_mode, set_pixel_format and set_size video source >operations with get_params display entity operation, as it was originally >in Laurent's version. > >We have discussed this matter on the mailing list and decided that it >would be better to have the source ask the sink for its parameter >structure and do whatever appropriate with it. Could you briefly outline the rationale behind this ? I'm wondering whether get_params could limit us if a device needs to modify parameters at runtime. For instance a panel might need to change clock frequencies or number of used data lane depending on various runtime conditions. I don't have a real use case here, so it might just be a bit of over-engineering. > - Defined a preliminary set of DSI bus parameters. > >Following parameters are defined: > >1. Pixel format used for video data transmission. >2. Mode flags (several bit flags ORed together): > a) DSI_MODE_VIDEO - entity uses video mode (as opposed to command > mode), following DSI_MODE_VIDEO_* flags are relevant only if this > flag is set. > b) DSI_MODE_VIDEO_BURST - entity uses burst transfer for video data > c) DSI_MODE_VIDEO_SYNC_PULSE - entity uses sync pulses (as opposed >to sync events) > d) DSI_MODE_VIDEO_AUTO_VERT - entity uses automatic video mode >detection > e) DSI_MODE_VIDEO_HSE - entity needs horizontal sync end packets > f) DSI_MODE_VIDEO_HFP - entity needs horizontal front porch area > g) DSI_MODE_VIDEO_HBP - entity needs horizontal back porch area > h) DSI_MODE_VIDEO_HSA - entity needs horizontal sync active area > i) DSI_MODE_VSYNC_FLUSH - vertical sync pulse flushes video data > j) DSI_MODE_EOT_PACKET - entity needs EoT packets >3. Bit (serial) clock frequency in HS mode. >4. Escape mode clock frequency. >5. Mask of used data lanes (each bit represents single lane). >From my experience with MIPI CSI (Camera Serial Interface, similar to DSI) some transceivers can reroute data lanes internally. Is that true for DSI as well ? In that case we would need a list of data lanes, not just a mask. >6. Command allow area in video mode - amount of lines after transmitting > video data when generic commands are accepted. > >Feel free to suggest anything missing or wrong, as the list of >parameters is based merely on what was used in original Exynos MIPI >DSIM driver. > > - Redesigned source-entity matching. > >Now each source has name string and integer id and each entity has >source name and source id. In addition, they can have of_node specified, >which is used for Device Tree-based matching. > >The matching procedure is as follows: > >1. Matching takes place when display entity registers. > a) If there is of_node specified for the entity then "video-source" > phandle of this node is being parsed and list of registered sources > is traversed in search of a source with of_node received from > parsing the phandle. > b) Otherwise the matching key is a pair of source name and id. >2. If no matching source is found, display_entity_register returns > -EPROBE_DEFER error which causes the entity driver to defer its > probe until the source registers. >3. Otherwise an optional bind operation of video source is called, > sink field of source and source field of entity are set and the > matching ends successfully. > > - Some initial concept of source-entity cross-locking. > >Only video source is protected at the moment, as I still have to think >a bit about locking the entity in a way where removing it by user request >is still possible. One of the main locking issues here are cross-references. The display entity holds a reference to the video source (for video operations), and the display controll
[Bug 60200] New: radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 Priority: medium Bug ID: 60200 Assignee: dri-devel@lists.freedesktop.org Summary: radeon_bo with virtual address referencing mismatch Severity: normal Classification: Unclassified OS: Linux (All) Reporter: g02ma...@gmail.com Hardware: x86-64 (AMD64) Status: NEW Version: git Component: Drivers/Gallium/r600 Product: Mesa Created attachment 74101 --> https://bugs.freedesktop.org/attachment.cgi?id=74101&action=edit patch First a disclaimer, this code is unfamiliar to me so there is probably stuff that i have misunderstood. When i try to start weston it immediatly crashes with the following error: radeon: Failed to allocate a buffer: radeon:size : 1 bytes radeon:alignment : 4096 bytes radeon:domains : 2 and dmesg says: [ 75.092178] radeon :01:00.0: bo 8802233f8800 va 0x conflict with (bo 8802219de400 0x0082C000 0x010F6000) [ 75.092195] radeon :01:00.0: bo 8802233f8800 don't has a mapping in vm 88022275ac00 Everything works fine if i disable the virtual address code, so the issue is in there somewhere. I tried to analyze it and this is what i came up with: Consider the following scenario: radeon_bomgr_create_bo: create gem buffer kernel: create buffer with handle 1 set ref count to 1 radeon_bomgr_create_bo: create radeon_bo with handle 1 set ref count to 1 radeon_bomgr_create_bo: find virtual address for handle 1 radeon_bomgr_find_va: free list empty use offset 2000 radeon_bomgr_create_bo: map virtual address for handle 1 at offset 2000 kernel: map virtual address for handle 1 at offset 2000 somewhere in userspace: create flink for handle 1 kernel: create flink name 1 for handle 1 radeon_winsys_bo_from_handle: open gem buffer with name 1 kernel: create handle 2 increase ref count to 2 radeon_winsys_bo_from_handle: create radeon_bo with handle 2 set ref count to 1 radeon_winsys_bo_from_handle: find virtual address for handle 2 radeon_bomgr_find_va: free list empty use offset 6000 radeon_winsys_bo_from_handle: map virtual address for handle 2 at offset 6000 kernel: virtual address already mapped for handle 2 use offset 2000 somewhere in userspace: destroy handle 2 somewhere in userspace: decrease handle 2 ref count to 0 radeon_bo_destroy: close gem buffer with handle 2 kernel: decrease ref count to 1 radeon_bo_destroy: free virtual address with offset 2000 radeon_bomgr_free_va: add virtual address with offset 2000 to free list radeon_bomgr_create_bo: create gem buffer kernel: create buffer with handle 3 set ref count to 1 radeon_bomgr_create_bo: create radeon_bo with handle 3 set ref count to 1 radeon_bomgr_create_bo: find virtual address for handle 3 radeon_bomgr_find_va: from free list use offset 2000 radeon_bomgr_create_bo: map virtual address for handle 3 at offset 2000 kernel: virtual address conflict offset 2000 already mapped This is a simplifed scenario what happens in my case. The issue, at least as i see it, is that the virtual address is freed(added to the free list) even though it is still mapped in the kernel. This is because the userspace referencing code does not track the radeon_bo object created in function radeon_bomgr_create_bo. The attached patch fixes the issue for me, but is probably not the correct fix. The issue was found and tested on wayland/weston master, mesa master and kernel 3.8.0-rc6 -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 60200] radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 --- Comment #1 from Martin Andersson --- Created attachment 74102 --> https://bugs.freedesktop.org/attachment.cgi?id=74102&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 60200] radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 --- Comment #2 from Martin Andersson --- Created attachment 74103 --> https://bugs.freedesktop.org/attachment.cgi?id=74103&action=edit weston -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 15/62] drm: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. * drm_ctxbitmap_next() error handling in drm_addctx() seems broken. drm_ctxbitmap_next() return -errno on failure not -1. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel@lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/drm_context.c | 17 +++-- drivers/gpu/drm/drm_crtc.c| 15 +++ drivers/gpu/drm/drm_gem.c | 35 +-- drivers/gpu/drm/drm_stub.c| 19 ++- 4 files changed, 21 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index 75f62c5..725968d 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -74,24 +74,13 @@ void drm_ctxbitmap_free(struct drm_device * dev, int ctx_handle) */ static int drm_ctxbitmap_next(struct drm_device * dev) { - int new_id; int ret; -again: - if (idr_pre_get(&dev->ctx_idr, GFP_KERNEL) == 0) { - DRM_ERROR("Out of memory expanding drawable idr\n"); - return -ENOMEM; - } mutex_lock(&dev->struct_mutex); - ret = idr_get_new_above(&dev->ctx_idr, NULL, - DRM_RESERVED_CONTEXTS, &new_id); + ret = idr_alloc(&dev->ctx_idr, NULL, DRM_RESERVED_CONTEXTS, 0, + GFP_KERNEL); mutex_unlock(&dev->struct_mutex); - if (ret == -EAGAIN) - goto again; - else if (ret) - return ret; - - return new_id; + return ret; } /** diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 9b39d1f..9da1f00 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -220,24 +220,15 @@ char *drm_get_connector_status_name(enum drm_connector_status status) static int drm_mode_object_get(struct drm_device *dev, struct drm_mode_object *obj, uint32_t obj_type) { - int new_id = 0; int ret; -again: - if (idr_pre_get(&dev->mode_config.crtc_idr, GFP_KERNEL) == 0) { - DRM_ERROR("Ran out memory getting a mode number\n"); - return -ENOMEM; - } - mutex_lock(&dev->mode_config.idr_mutex); - ret = idr_get_new_above(&dev->mode_config.crtc_idr, obj, 1, &new_id); + ret = idr_alloc(&dev->mode_config.crtc_idr, obj, 1, 0, GFP_KERNEL); mutex_unlock(&dev->mode_config.idr_mutex); - if (ret == -EAGAIN) - goto again; - else if (ret) + if (ret < 0) return ret; - obj->id = new_id; + obj->id = ret; obj->type = obj_type; return 0; } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index e775859..6577514 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -270,21 +270,19 @@ drm_gem_handle_create(struct drm_file *file_priv, int ret; /* -* Get the user-visible handle using idr. +* Get the user-visible handle using idr. Preload and perform +* allocation under our spinlock. */ -again: - /* ensure there is space available to allocate a handle */ - if (idr_pre_get(&file_priv->object_idr, GFP_KERNEL) == 0) - return -ENOMEM; - - /* do the allocation under our spinlock */ + idr_preload(GFP_KERNEL); spin_lock(&file_priv->table_lock); - ret = idr_get_new_above(&file_priv->object_idr, obj, 1, (int *)handlep); + + ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT); + spin_unlock(&file_priv->table_lock); - if (ret == -EAGAIN) - goto again; - else if (ret) + idr_preload_end(); + if (ret < 0) return ret; + *handlep = ret; drm_gem_object_handle_reference(obj); @@ -451,22 +449,15 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, if (obj == NULL) return -ENOENT; -again: - if (idr_pre_get(&dev->object_name_idr, GFP_KERNEL) == 0) { - ret = -ENOMEM; - goto err; - } - + idr_preload(GFP_KERNEL); spin_lock(&dev->object_name_lock); if (!obj->name) { - ret = idr_get_new_above(&dev->object_name_idr, obj, 1, - &obj->name); + ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT); + obj->name = ret; args->name = (uint64_t) obj->name; spin_unlock(&dev->object_name_lock); - if (ret == -EAGAIN) - goto again; - else if (ret) + if (ret < 0) goto err; /* Allocate a reference for the name table. */ diff --git a/driver
[PATCH 16/62] drm/exynos: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: Kukjin Kim Cc: dri-devel@lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/exynos/exynos_drm_ipp.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 49278f0..62c24fb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -137,21 +137,15 @@ static int ipp_create_id(struct idr *id_idr, struct mutex *lock, void *obj, DRM_DEBUG_KMS("%s\n", __func__); -again: - /* ensure there is space available to allocate a handle */ - if (idr_pre_get(id_idr, GFP_KERNEL) == 0) { - DRM_ERROR("failed to get idr.\n"); - return -ENOMEM; - } - /* do the allocation under our mutexlock */ mutex_lock(lock); - ret = idr_get_new_above(id_idr, obj, 1, (int *)idp); + ret = idr_alloc(id_idr, obj, 1, 0, GFP_KERNEL); mutex_unlock(lock); - if (ret == -EAGAIN) - goto again; + if (ret < 0) + return ret; - return ret; + *idp = ret; + return 0; } static void *ipp_find_obj(struct idr *id_idr, struct mutex *lock, u32 id) -- 1.8.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 17/62] drm/i915: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/i915/i915_gem_context.c | 21 + 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a3f06bc..27e586a 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -144,7 +144,7 @@ create_hw_context(struct drm_device *dev, { struct drm_i915_private *dev_priv = dev->dev_private; struct i915_hw_context *ctx; - int ret, id; + int ret; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (ctx == NULL) @@ -169,22 +169,11 @@ create_hw_context(struct drm_device *dev, ctx->file_priv = file_priv; -again: - if (idr_pre_get(&file_priv->context_idr, GFP_KERNEL) == 0) { - ret = -ENOMEM; - DRM_DEBUG_DRIVER("idr allocation failed\n"); - goto err_out; - } - - ret = idr_get_new_above(&file_priv->context_idr, ctx, - DEFAULT_CONTEXT_ID + 1, &id); - if (ret == 0) - ctx->id = id; - - if (ret == -EAGAIN) - goto again; - else if (ret) + ret = idr_alloc(&file_priv->context_idr, ctx, DEFAULT_CONTEXT_ID + 1, 0, + GFP_KERNEL); + if (ret < 0) goto err_out; + ctx->id = ret; return ctx; -- 1.8.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 18/62] drm/sis: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel@lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/sis/sis_mm.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index 2b2f78c..9a43d98 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -128,17 +128,10 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file, if (retval) goto fail_alloc; -again: - if (idr_pre_get(&dev_priv->object_idr, GFP_KERNEL) == 0) { - retval = -ENOMEM; - goto fail_idr; - } - - retval = idr_get_new_above(&dev_priv->object_idr, item, 1, &user_key); - if (retval == -EAGAIN) - goto again; - if (retval) + retval = idr_alloc(&dev_priv->object_idr, item, 1, 0, GFP_KERNEL); + if (retval < 0) goto fail_idr; + user_key = retval; list_add(&item->owner_list, &file_priv->obj_list); mutex_unlock(&dev->struct_mutex); -- 1.8.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 19/62] drm/via: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel@lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/via/via_mm.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 0d55432..0ab93ff 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -148,17 +148,10 @@ int via_mem_alloc(struct drm_device *dev, void *data, if (retval) goto fail_alloc; -again: - if (idr_pre_get(&dev_priv->object_idr, GFP_KERNEL) == 0) { - retval = -ENOMEM; - goto fail_idr; - } - - retval = idr_get_new_above(&dev_priv->object_idr, item, 1, &user_key); - if (retval == -EAGAIN) - goto again; - if (retval) + retval = idr_alloc(&dev_priv->object_idr, item, 1, 0, GFP_KERNEL); + if (retval < 0) goto fail_idr; + user_key = retval; list_add(&item->owner_list, &file_priv->obj_list); mutex_unlock(&dev->struct_mutex); -- 1.8.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 20/62] drm/vmwgfx: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel@lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index e01a17b..c9d0676 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -177,17 +177,16 @@ int vmw_resource_alloc_id(struct vmw_resource *res) BUG_ON(res->id != -1); - do { - if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0)) - return -ENOMEM; - - write_lock(&dev_priv->resource_lock); - ret = idr_get_new_above(idr, res, 1, &res->id); - write_unlock(&dev_priv->resource_lock); + idr_preload(GFP_KERNEL); + write_lock(&dev_priv->resource_lock); - } while (ret == -EAGAIN); + ret = idr_alloc(idr, res, 1, 0, GFP_NOWAIT); + if (ret >= 0) + res->id = ret; - return ret; + write_unlock(&dev_priv->resource_lock); + idr_preload_end(); + return ret < 0 ? ret : 0; } /** -- 1.8.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 59903] [RS880] Xorg.0.log: flip queue failed: Device or resource busy
https://bugs.freedesktop.org/show_bug.cgi?id=59903 klondike changed: What|Removed |Added CC||klond...@klondike.es --- Comment #12 from klondike --- I have found this issue today at FOSDEM after disconnecting the VGA cable from the projector (not sure if before or after putting the computer to sleep). When I came back, this wasw happening and xrandr showed what follows: Screen 0: minimum 320 x 200, current 1366 x 768, maximum 8192 x 8192 LVDS connected 1366x768+0+0 (normal left inverted right x axis y axis) 0mm x 0mm 1366x768 59.9*+ 1280x720 59.9 1152x768 59.8 1024x768 59.9 800x60059.9 848x48059.7 720x48059.7 640x48059.4 HDMI-0 disconnected (normal left inverted right x axis y axis) VGA-0 disconnected 1024x768+0+0 (normal left inverted right x axis y axis) 0mm x 0mm 1024x768 (0x6c77) 65.0MHz h: width 1024 start 1048 end 1184 total 1344 skew0 clock 48.4KHz v: height 768 start 771 end 777 total 806 clock 60.0Hz After running xrandr --output VGA-0 --off to turn VGA off it worked again. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/tegra: Implement page-flipping support
On Wednesday 16 January 2013 13:36:17 Daniel Vetter wrote: > On Wed, Jan 16, 2013 at 11:01 AM, Thierry Reding wrote: > > drm_events_release() should be enough to clean up the events, but I > > suspect the reason why Laurent put that code in was that the drm_crtc > > private data still has a reference to the event and needs to clear it. > > Otherwise the next page flip won't be scheduled because .page_flip() > > would return -EBUSY. > > Hm, indeed we seem to have a nice bug in most drivers there :( That's not the only reason. drm_events_release() handles vblank events added to the vblank_event_list. That list only contains vblank events added by drm_queue_vblank_event(), only called by drm_wait_vblank(). Page flip events never get there, so they need to be cleaned up manually. I wrote in the DRM documentation, in the page flipping section, "FIXME: Could drivers that don't need to wait for rendering to complete just add the event to dev->vblank_event_list and let the DRM core handle everything, as for "normal" vertical blanking events?" We should investigate that. > > However, it seems like {tegra_dc,shmob_drm_crtc}_cancel_page_flip() > > could both be simplified a lot and just set their event to NULL. Then > > again, maybe keeping a separate reference isn't all that useful. Maybe > > the better thing to do here is iterate over the list of pending VBLANK > > events in *_finish_page_flip() and process each of them? That would > > allow more than one user-space process to queue page flips. > > I think we need a slightly more generally useful solution, since most > drivers are currently broken. I've read a bit through the code, but > short of refcounting drm events and adding event->file_priv checks at > relevent places I don't see a sane solution ... And even that one is > rather invasive. Do you have an idea? Imo doing the cleanup in each > driver will be rather error-prone, and since usually kms clients wait > for flips to complete, also guaranteed to be little tested. -- Regards, Laurent Pinchart
[PATCH 5/5] drm/tegra: Implement page-flipping support
Hi Thierry, On Wednesday 16 January 2013 19:56:06 Thierry Reding wrote: > On Wed, Jan 16, 2013 at 01:36:17PM +0100, Daniel Vetter wrote: > > On Wed, Jan 16, 2013 at 11:01 AM, Thierry Reding wrote: > > > drm_events_release() should be enough to clean up the events, but I > > > suspect the reason why Laurent put that code in was that the drm_crtc > > > private data still has a reference to the event and needs to clear it. > > > Otherwise the next page flip won't be scheduled because .page_flip() > > > would return -EBUSY. > > > > Hm, indeed we seem to have a nice bug in most drivers there :( > > > > > However, it seems like {tegra_dc,shmob_drm_crtc}_cancel_page_flip() > > > could both be simplified a lot and just set their event to NULL. Then > > > again, maybe keeping a separate reference isn't all that useful. Maybe > > > the better thing to do here is iterate over the list of pending VBLANK > > > events in *_finish_page_flip() and process each of them? That would > > > allow more than one user-space process to queue page flips. > > > > I think we need a slightly more generally useful solution, since most > > drivers are currently broken. I've read a bit through the code, but > > short of refcounting drm events and adding event->file_priv checks at > > relevent places I don't see a sane solution ... And even that one is > > rather invasive. Do you have an idea? Imo doing the cleanup in each > > driver will be rather error-prone, and since usually kms clients wait > > for flips to complete, also guaranteed to be little tested. > > While this probably doesn't improve the situation much, maybe adding > more extensive tests to libdrm or so would help. I wrote a couple of > small programs to test vblank and plane support. > > The vblank one basically generates two framebuffers with different > patterns and uses page-flipping to alternate between them. The plane > test does something similar, sets up a plane and associates a buffer > with it. It includes scaling the plane to test that functionality as > well. > > Perhaps these tests could even be added to the existing libdrm tests, > but maybe having separate binaries wouldn't hurt. Further cleanup of the modetest application is somewhere on my to-do list (but probably so low that I'll never get to it unless there's a real incentive ;-)). It's a good candidate for more page flip testing (there's basic page flip support there already). > Back to the original topic: should it not work to queue a page flip and > immediately exit(0) after that to test the cleanup code? I suppose if > that can be tested on all drivers we should have pretty good coverage. Maybe we need some kind of compliance test suite tool ? -- Regards, Laurent Pinchart -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 490 bytes Desc: This is a digitally signed message part. URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/fd395da7/attachment.pgp>
[RFC v2 0/5] Common Display Framework
Hi Marcus, On Tuesday 08 January 2013 18:08:19 Marcus Lorentzon wrote: > On 01/08/2013 05:36 PM, Tomasz Figa wrote: > > On Tuesday 08 of January 2013 11:12:26 Marcus Lorentzon wrote: [snip] > >> FYI, > >> here is STE "DSI API": > >> http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f > >> =include/video/mcde.h;h=499ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD > >> #l361 Thank you. > >> But it is not perfect. After a couple of products we realized that most > >> panel drivers want an easy way to send a bunch of init commands in one > >> go. So I think it should be an op for sending an array of commands at > >> once. Something like > >> > >> struct dsi_cmd { > >> enum mipi_pkt_type type; /* MIPI DSI, DCS, SetPacketLen, ... */ > >> u8 cmd; > >> int dataLen; > >> u8 *data; > >> } > >> > >> struct dsi_ops { > >> int dsi_write(source, int num_cmds, struct dsi_cmd *cmds); > >> ... > >> } Do you have DSI IP(s) that can handle a list of commands ? Or would all DSI transmitter drivers need to iterate over the commands manually ? In the later case a lower-level API might be easier to implement in DSI transmitter drivers. Helper functions could provide the higher-level API you proposed. > > Yes, this should be flexible enough to cover most of (or even whole) DSI > > specification. > > > > However I'm not sure whether the dsi_write name would be appropriate, > > since DSI packet types include also read and special transactions. So, > > according to DSI terminology, maybe dsi_transaction would be better? Or dsi_transfer or dsi_xfer ? Does the DSI bus have a concept of transactions ? > I think read should still be separate. At least on my HW read and write > are quite different. But all "transactions" are very much the same in HW > setup. The ... was dsi_write etc ;) Like set_max_packet_size should > maybe be an ops. Since only the implementer of the "video source" will > know what the max read return packet size for that DSI IP is. The panels > don't know that. Maybe another ops to retrieve some info about the caps > of the video source would help that. Then a helper could call that and > then the dsi_write one. If panels (or helper functions) need information about the DSI transmitter capabilities, sure, we can add an op. > >> And I think I still prefer the dsi_bus in favor of the abstract video > >> source. It just looks like a home made bus with bus-ops ... can't you do > >> something similar using the normal driver framework? enable/disable > >> looks like suspend/resume, register/unregister_vid_src is like > >> bus_(un)register_device, ... the video source anyway seems unattached > >> to the panel stuff with the find_video_source call. The Linux driver framework is based on control busses. DSI usually handles both control and video transfer, but the control and data busses can also be separate (think DPI + SPI/I2C for instance). In that case the panel will be a child of its control bus master, and will need a separate interface to access video data operations. As a separate video interface is thus needed, I think we should use it for DSI as well. My initial proposal included a DBI bus (as I don't have any DSI hardware - DBI and DSI can be used interchangeably in this discussions, they both share the caracteristic of having a common control + data bus), and panels were children of the DBI bus master. The DBI bus API was only used for control, not for data transfers. Tomi then removed the DBI bus and moved the control operations to the video source, turning the DBI panels into platform devices. I still favor my initial approach, but I can agree to drop the DBI bus if there's a consensus on that. Video bus operations will be separate in any case. > > DSI needs specific power management. It's necessary to power up the panel > > first to make it wait for Tinit event and then enable DSI master to > > trigger such event. Only then rest of panel initialization can be > > completed. > > I know, we have a very complex sequence for our HDMI encoder which uses > sort of continuous DSI cmmand mode. And power/clock on sequences are > tricky to get right in our current "CDF" API (mcde_display). But I fail > to see how the current video source API is different from just using the > bus/device APIs. As mentioned above, the video source API handles video transfers, while the bus/device API handles control transfers. Operations such as "start the video stream" will thus be video source APIs. Operations such as "enable the DSI master", used to trigger the Tinit event (whatever that is :-)) at power up time would probably be DSI bus operations. > > Also, as discussed in previous posts, some panels might use DSI only for > > video data and another interface (I2C, SPI) for control data. In such case > > it would be impossible to represent such device in a reasonable way using > > current driver model. > > I understand that you need to get h
[RFC v2 0/5] Common Display Framework
Hi Marcus, On Tuesday 08 January 2013 11:12:26 Marcus Lorentzon wrote: [snip] > I also looked at the video source in Tomi's git tree > (http://gitorious.org/linux-omap-dss2/linux/blobs/work/dss-dev-model-cdf/inc > lude/video/display.h). I think I would prefer a single "setup" op taking a > "struct dsi_config" as argument. Then each DSI formatter/encoder driver > could decide best way to set that up. We have something similar at > http://www.igloocommunity.org/gitweb/?p=kernel/igloo-kernel.git;a=blob;f=inc > lude/video/mcde.h;h=499ce5cfecc9ad77593e761cdcc1624502f28432;hb=HEAD#l118 A single setup function indeed seems easier, but I don't have enough experience with DSI to have a strong opinion on that. We'll have to compare implementations if there's a disagreement on this. -- Regards, Laurent Pinchart
[RFC v2 0/5] Common Display Framework
Hi Rahul, On Wednesday 09 January 2013 13:53:30 Rahul Sharma wrote: > Hi Laurent, > > CDF will also be helpful in supporting Panels with integrated audio > (HDMI/DP) if we can add audio related control operations to > display_entity_control_ops. Video controls will be called by crtc in DRM/V4L > and audio controls from Alsa. I knew that would come up at some point :-) I agree with you that adding audio support would be a very nice improvement, and I'm totally open to that, but I will concentrate on video, at least to start with. The first reason is that I'm not familiar enough with ALSA, and the second that there's only 24h per day :-) Please feel free, of course, to submit a proposal for audio support. > Secondly, if I need to support get_modes operation in hdmi/dp panel, I need > to implement edid parser inside the panel driver. It will be meaningful to > add get_edid control operation for hdmi/dp. Even if EDID data is parsed in the panel driver, raw EDID will still need to be exported, so a get_edid control operation (or something similar) is definitely needed. There's no disagreement on this, I just haven't included that operation yet because my test hardware is purely panel-based. -- Regards, Laurent Pinchart
[Bug 60182] New: X.Org Server terminate when I close video player
https://bugs.freedesktop.org/show_bug.cgi?id=60182 Priority: medium Bug ID: 60182 Assignee: dri-devel at lists.freedesktop.org Summary: X.Org Server terminate when I close video player Severity: critical Classification: Unclassified OS: Linux (All) Reporter: runetmember at gmail.com Hardware: x86-64 (AMD64) Status: NEW Version: unspecified Component: DRM/Radeon Product: DRI X.Org Server terminate when I close VLC (OpenGL GLX video output). This issue not reproducible when VLC doesn't playback anything. So only closing player that still playback video trigger issue. Issue reproducible not every time (around 1 of 10 closing VLC get X.Org Server termination). Issue tested only on integrated GPU (Radeon HD 6620G). Kubuntu 13.04 x86_64 Linux 3.8rc6 (with Linux 3.7 is also reproducible). Mesa: 9.0.2 (with 9.1 is also reproducible) libdrm-radeon1: 2.4.41 xserver-xorg-video-radeon: 7.1.0 xserver-xorg-core: 1.13.2 KDE 4.9.98 (4.10rc3) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/ec2c2ae7/attachment.html>
[Bug 60182] X.Org Server terminate when I close video player
https://bugs.freedesktop.org/show_bug.cgi?id=60182 --- Comment #1 from runetmember at gmail.com --- Created attachment 74078 --> https://bugs.freedesktop.org/attachment.cgi?id=74078&action=edit Log of terminated X.Org Server -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/e90b4e14/attachment.html>
[Bug 60182] X.Org Server terminate when I close video player
https://bugs.freedesktop.org/show_bug.cgi?id=60182 --- Comment #2 from runetmember at gmail.com --- Created attachment 74079 --> https://bugs.freedesktop.org/attachment.cgi?id=74079&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/998eafe2/attachment.html>
[Bug 60182] X.Org Server terminate when I close video player
https://bugs.freedesktop.org/show_bug.cgi?id=60182 --- Comment #3 from runetmember at gmail.com --- Also a little bit info, just in case: Exact GPU model: 1002:6741 Advanced Micro Devices [AMD] nee ATI BeaverCreek [Mobility Radeon HD 6620G] VLC version: 2.0.5. If I need to provide any additional information, please let me know. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/d32c9b67/attachment.html>
[RFC v2 0/5] Common Display Framework
On Fri, Feb 1, 2013 at 5:42 PM, Laurent Pinchart wrote: > Hi Rahul, > > On Wednesday 09 January 2013 13:53:30 Rahul Sharma wrote: >> Hi Laurent, >> >> CDF will also be helpful in supporting Panels with integrated audio >> (HDMI/DP) if we can add audio related control operations to >> display_entity_control_ops. Video controls will be called by crtc in DRM/V4L >> and audio controls from Alsa. > > I knew that would come up at some point :-) I agree with you that adding audio > support would be a very nice improvement, and I'm totally open to that, but I > will concentrate on video, at least to start with. The first reason is that > I'm not familiar enough with ALSA, and the second that there's only 24h per > day :-) > > Please feel free, of course, to submit a proposal for audio support. > >> Secondly, if I need to support get_modes operation in hdmi/dp panel, I need >> to implement edid parser inside the panel driver. It will be meaningful to >> add get_edid control operation for hdmi/dp. > > Even if EDID data is parsed in the panel driver, raw EDID will still need to > be exported, so a get_edid control operation (or something similar) is > definitely needed. There's no disagreement on this, I just haven't included > that operation yet because my test hardware is purely panel-based. one of (probably many) places that just keeping CDF (CDH? common display helpers..) inside DRM makes life easier :-P BR, -R > -- > Regards, > > Laurent Pinchart > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/4] Common Display Framework-TF
Hi Tomasz, Thank you for your RFC. On Wednesday 30 January 2013 16:38:59 Tomasz Figa wrote: > Hi, > > After pretty long time of trying to adapt Exynos-specific DSI display > support to Common Display Framework I'm ready to show some preliminary RFC > patches. This series shows some ideas for CDF that came to my mind during > my work, some changes based on comments received by Tomi's edition of CDF > and also preliminarys version of Exynos DSI (video source part only, still > with some FIXMEs) and Samsung S6E8AX0 DSI panel drivers. > > It is heavily based on Tomi's work which can be found here: > http://thread.gmane.org/gmane.comp.video.dri.devel/78227 > > The code might be a bit hacky in places, as I wanted to get it to a kind > of complete and working state first. However I hope that some of the ideas > might useful for further works. > > So, here comes the TF edition of Common Clock Framework. > > > Changes done to Tomi's edition of CDF: > > - Replaced set_operation_mode, set_pixel_format and set_size video source >operations with get_params display entity operation, as it was originally >in Laurent's version. > >We have discussed this matter on the mailing list and decided that it >would be better to have the source ask the sink for its parameter >structure and do whatever appropriate with it. Could you briefly outline the rationale behind this ? I'm wondering whether get_params could limit us if a device needs to modify parameters at runtime. For instance a panel might need to change clock frequencies or number of used data lane depending on various runtime conditions. I don't have a real use case here, so it might just be a bit of over-engineering. > - Defined a preliminary set of DSI bus parameters. > >Following parameters are defined: > >1. Pixel format used for video data transmission. >2. Mode flags (several bit flags ORed together): > a) DSI_MODE_VIDEO - entity uses video mode (as opposed to command > mode), following DSI_MODE_VIDEO_* flags are relevant only if this > flag is set. > b) DSI_MODE_VIDEO_BURST - entity uses burst transfer for video data > c) DSI_MODE_VIDEO_SYNC_PULSE - entity uses sync pulses (as opposed >to sync events) > d) DSI_MODE_VIDEO_AUTO_VERT - entity uses automatic video mode >detection > e) DSI_MODE_VIDEO_HSE - entity needs horizontal sync end packets > f) DSI_MODE_VIDEO_HFP - entity needs horizontal front porch area > g) DSI_MODE_VIDEO_HBP - entity needs horizontal back porch area > h) DSI_MODE_VIDEO_HSA - entity needs horizontal sync active area > i) DSI_MODE_VSYNC_FLUSH - vertical sync pulse flushes video data > j) DSI_MODE_EOT_PACKET - entity needs EoT packets >3. Bit (serial) clock frequency in HS mode. >4. Escape mode clock frequency. >5. Mask of used data lanes (each bit represents single lane). >From my experience with MIPI CSI (Camera Serial Interface, similar to DSI) some transceivers can reroute data lanes internally. Is that true for DSI as well ? In that case we would need a list of data lanes, not just a mask. >6. Command allow area in video mode - amount of lines after transmitting > video data when generic commands are accepted. > >Feel free to suggest anything missing or wrong, as the list of >parameters is based merely on what was used in original Exynos MIPI >DSIM driver. > > - Redesigned source-entity matching. > >Now each source has name string and integer id and each entity has >source name and source id. In addition, they can have of_node specified, >which is used for Device Tree-based matching. > >The matching procedure is as follows: > >1. Matching takes place when display entity registers. > a) If there is of_node specified for the entity then "video-source" > phandle of this node is being parsed and list of registered sources > is traversed in search of a source with of_node received from > parsing the phandle. > b) Otherwise the matching key is a pair of source name and id. >2. If no matching source is found, display_entity_register returns > -EPROBE_DEFER error which causes the entity driver to defer its > probe until the source registers. >3. Otherwise an optional bind operation of video source is called, > sink field of source and source field of entity are set and the > matching ends successfully. > > - Some initial concept of source-entity cross-locking. > >Only video source is protected at the moment, as I still have to think >a bit about locking the entity in a way where removing it by user request >is still possible. One of the main locking issues here are cross-references. The display entity holds a reference to the video source (for video operations), and the display controll
[Bug 60200] New: radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 Priority: medium Bug ID: 60200 Assignee: dri-devel at lists.freedesktop.org Summary: radeon_bo with virtual address referencing mismatch Severity: normal Classification: Unclassified OS: Linux (All) Reporter: g02maran at gmail.com Hardware: x86-64 (AMD64) Status: NEW Version: git Component: Drivers/Gallium/r600 Product: Mesa Created attachment 74101 --> https://bugs.freedesktop.org/attachment.cgi?id=74101&action=edit patch First a disclaimer, this code is unfamiliar to me so there is probably stuff that i have misunderstood. When i try to start weston it immediatly crashes with the following error: radeon: Failed to allocate a buffer: radeon:size : 1 bytes radeon:alignment : 4096 bytes radeon:domains : 2 and dmesg says: [ 75.092178] radeon :01:00.0: bo 8802233f8800 va 0x conflict with (bo 8802219de400 0x0082C000 0x010F6000) [ 75.092195] radeon :01:00.0: bo 8802233f8800 don't has a mapping in vm 88022275ac00 Everything works fine if i disable the virtual address code, so the issue is in there somewhere. I tried to analyze it and this is what i came up with: Consider the following scenario: radeon_bomgr_create_bo: create gem buffer kernel: create buffer with handle 1 set ref count to 1 radeon_bomgr_create_bo: create radeon_bo with handle 1 set ref count to 1 radeon_bomgr_create_bo: find virtual address for handle 1 radeon_bomgr_find_va: free list empty use offset 2000 radeon_bomgr_create_bo: map virtual address for handle 1 at offset 2000 kernel: map virtual address for handle 1 at offset 2000 somewhere in userspace: create flink for handle 1 kernel: create flink name 1 for handle 1 radeon_winsys_bo_from_handle: open gem buffer with name 1 kernel: create handle 2 increase ref count to 2 radeon_winsys_bo_from_handle: create radeon_bo with handle 2 set ref count to 1 radeon_winsys_bo_from_handle: find virtual address for handle 2 radeon_bomgr_find_va: free list empty use offset 6000 radeon_winsys_bo_from_handle: map virtual address for handle 2 at offset 6000 kernel: virtual address already mapped for handle 2 use offset 2000 somewhere in userspace: destroy handle 2 somewhere in userspace: decrease handle 2 ref count to 0 radeon_bo_destroy: close gem buffer with handle 2 kernel: decrease ref count to 1 radeon_bo_destroy: free virtual address with offset 2000 radeon_bomgr_free_va: add virtual address with offset 2000 to free list radeon_bomgr_create_bo: create gem buffer kernel: create buffer with handle 3 set ref count to 1 radeon_bomgr_create_bo: create radeon_bo with handle 3 set ref count to 1 radeon_bomgr_create_bo: find virtual address for handle 3 radeon_bomgr_find_va: from free list use offset 2000 radeon_bomgr_create_bo: map virtual address for handle 3 at offset 2000 kernel: virtual address conflict offset 2000 already mapped This is a simplifed scenario what happens in my case. The issue, at least as i see it, is that the virtual address is freed(added to the free list) even though it is still mapped in the kernel. This is because the userspace referencing code does not track the radeon_bo object created in function radeon_bomgr_create_bo. The attached patch fixes the issue for me, but is probably not the correct fix. The issue was found and tested on wayland/weston master, mesa master and kernel 3.8.0-rc6 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/7f3b88fd/attachment.html>
[Bug 60200] radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 --- Comment #1 from Martin Andersson --- Created attachment 74102 --> https://bugs.freedesktop.org/attachment.cgi?id=74102&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/a6ca9117/attachment.html>
[Bug 60200] radeon_bo with virtual address referencing mismatch
https://bugs.freedesktop.org/show_bug.cgi?id=60200 --- Comment #2 from Martin Andersson --- Created attachment 74103 --> https://bugs.freedesktop.org/attachment.cgi?id=74103&action=edit weston -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130202/f2d1d954/attachment.html>
[PATCH 15/62] drm: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. * drm_ctxbitmap_next() error handling in drm_addctx() seems broken. drm_ctxbitmap_next() return -errno on failure not -1. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel at lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/drm_context.c | 17 +++-- drivers/gpu/drm/drm_crtc.c| 15 +++ drivers/gpu/drm/drm_gem.c | 35 +-- drivers/gpu/drm/drm_stub.c| 19 ++- 4 files changed, 21 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index 75f62c5..725968d 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -74,24 +74,13 @@ void drm_ctxbitmap_free(struct drm_device * dev, int ctx_handle) */ static int drm_ctxbitmap_next(struct drm_device * dev) { - int new_id; int ret; -again: - if (idr_pre_get(&dev->ctx_idr, GFP_KERNEL) == 0) { - DRM_ERROR("Out of memory expanding drawable idr\n"); - return -ENOMEM; - } mutex_lock(&dev->struct_mutex); - ret = idr_get_new_above(&dev->ctx_idr, NULL, - DRM_RESERVED_CONTEXTS, &new_id); + ret = idr_alloc(&dev->ctx_idr, NULL, DRM_RESERVED_CONTEXTS, 0, + GFP_KERNEL); mutex_unlock(&dev->struct_mutex); - if (ret == -EAGAIN) - goto again; - else if (ret) - return ret; - - return new_id; + return ret; } /** diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 9b39d1f..9da1f00 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -220,24 +220,15 @@ char *drm_get_connector_status_name(enum drm_connector_status status) static int drm_mode_object_get(struct drm_device *dev, struct drm_mode_object *obj, uint32_t obj_type) { - int new_id = 0; int ret; -again: - if (idr_pre_get(&dev->mode_config.crtc_idr, GFP_KERNEL) == 0) { - DRM_ERROR("Ran out memory getting a mode number\n"); - return -ENOMEM; - } - mutex_lock(&dev->mode_config.idr_mutex); - ret = idr_get_new_above(&dev->mode_config.crtc_idr, obj, 1, &new_id); + ret = idr_alloc(&dev->mode_config.crtc_idr, obj, 1, 0, GFP_KERNEL); mutex_unlock(&dev->mode_config.idr_mutex); - if (ret == -EAGAIN) - goto again; - else if (ret) + if (ret < 0) return ret; - obj->id = new_id; + obj->id = ret; obj->type = obj_type; return 0; } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index e775859..6577514 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -270,21 +270,19 @@ drm_gem_handle_create(struct drm_file *file_priv, int ret; /* -* Get the user-visible handle using idr. +* Get the user-visible handle using idr. Preload and perform +* allocation under our spinlock. */ -again: - /* ensure there is space available to allocate a handle */ - if (idr_pre_get(&file_priv->object_idr, GFP_KERNEL) == 0) - return -ENOMEM; - - /* do the allocation under our spinlock */ + idr_preload(GFP_KERNEL); spin_lock(&file_priv->table_lock); - ret = idr_get_new_above(&file_priv->object_idr, obj, 1, (int *)handlep); + + ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT); + spin_unlock(&file_priv->table_lock); - if (ret == -EAGAIN) - goto again; - else if (ret) + idr_preload_end(); + if (ret < 0) return ret; + *handlep = ret; drm_gem_object_handle_reference(obj); @@ -451,22 +449,15 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, if (obj == NULL) return -ENOENT; -again: - if (idr_pre_get(&dev->object_name_idr, GFP_KERNEL) == 0) { - ret = -ENOMEM; - goto err; - } - + idr_preload(GFP_KERNEL); spin_lock(&dev->object_name_lock); if (!obj->name) { - ret = idr_get_new_above(&dev->object_name_idr, obj, 1, - &obj->name); + ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT); + obj->name = ret; args->name = (uint64_t) obj->name; spin_unlock(&dev->object_name_lock); - if (ret == -EAGAIN) - goto again; - else if (ret) + if (ret < 0) goto err; /* Allocate a reference for the name table. */ diff --git a/drivers/gpu/d
[PATCH 16/62] drm/exynos: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: Kukjin Kim Cc: dri-devel at lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/exynos/exynos_drm_ipp.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 49278f0..62c24fb 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -137,21 +137,15 @@ static int ipp_create_id(struct idr *id_idr, struct mutex *lock, void *obj, DRM_DEBUG_KMS("%s\n", __func__); -again: - /* ensure there is space available to allocate a handle */ - if (idr_pre_get(id_idr, GFP_KERNEL) == 0) { - DRM_ERROR("failed to get idr.\n"); - return -ENOMEM; - } - /* do the allocation under our mutexlock */ mutex_lock(lock); - ret = idr_get_new_above(id_idr, obj, 1, (int *)idp); + ret = idr_alloc(id_idr, obj, 1, 0, GFP_KERNEL); mutex_unlock(lock); - if (ret == -EAGAIN) - goto again; + if (ret < 0) + return ret; - return ret; + *idp = ret; + return 0; } static void *ipp_find_obj(struct idr *id_idr, struct mutex *lock, u32 id) -- 1.8.1
[PATCH 17/62] drm/i915: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel at lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/i915/i915_gem_context.c | 21 + 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a3f06bc..27e586a 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -144,7 +144,7 @@ create_hw_context(struct drm_device *dev, { struct drm_i915_private *dev_priv = dev->dev_private; struct i915_hw_context *ctx; - int ret, id; + int ret; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (ctx == NULL) @@ -169,22 +169,11 @@ create_hw_context(struct drm_device *dev, ctx->file_priv = file_priv; -again: - if (idr_pre_get(&file_priv->context_idr, GFP_KERNEL) == 0) { - ret = -ENOMEM; - DRM_DEBUG_DRIVER("idr allocation failed\n"); - goto err_out; - } - - ret = idr_get_new_above(&file_priv->context_idr, ctx, - DEFAULT_CONTEXT_ID + 1, &id); - if (ret == 0) - ctx->id = id; - - if (ret == -EAGAIN) - goto again; - else if (ret) + ret = idr_alloc(&file_priv->context_idr, ctx, DEFAULT_CONTEXT_ID + 1, 0, + GFP_KERNEL); + if (ret < 0) goto err_out; + ctx->id = ret; return ctx; -- 1.8.1
[PATCH 18/62] drm/sis: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel at lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/sis/sis_mm.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/sis/sis_mm.c b/drivers/gpu/drm/sis/sis_mm.c index 2b2f78c..9a43d98 100644 --- a/drivers/gpu/drm/sis/sis_mm.c +++ b/drivers/gpu/drm/sis/sis_mm.c @@ -128,17 +128,10 @@ static int sis_drm_alloc(struct drm_device *dev, struct drm_file *file, if (retval) goto fail_alloc; -again: - if (idr_pre_get(&dev_priv->object_idr, GFP_KERNEL) == 0) { - retval = -ENOMEM; - goto fail_idr; - } - - retval = idr_get_new_above(&dev_priv->object_idr, item, 1, &user_key); - if (retval == -EAGAIN) - goto again; - if (retval) + retval = idr_alloc(&dev_priv->object_idr, item, 1, 0, GFP_KERNEL); + if (retval < 0) goto fail_idr; + user_key = retval; list_add(&item->owner_list, &file_priv->obj_list); mutex_unlock(&dev->struct_mutex); -- 1.8.1
[PATCH 19/62] drm/via: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel at lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/via/via_mm.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/via/via_mm.c b/drivers/gpu/drm/via/via_mm.c index 0d55432..0ab93ff 100644 --- a/drivers/gpu/drm/via/via_mm.c +++ b/drivers/gpu/drm/via/via_mm.c @@ -148,17 +148,10 @@ int via_mem_alloc(struct drm_device *dev, void *data, if (retval) goto fail_alloc; -again: - if (idr_pre_get(&dev_priv->object_idr, GFP_KERNEL) == 0) { - retval = -ENOMEM; - goto fail_idr; - } - - retval = idr_get_new_above(&dev_priv->object_idr, item, 1, &user_key); - if (retval == -EAGAIN) - goto again; - if (retval) + retval = idr_alloc(&dev_priv->object_idr, item, 1, 0, GFP_KERNEL); + if (retval < 0) goto fail_idr; + user_key = retval; list_add(&item->owner_list, &file_priv->obj_list); mutex_unlock(&dev->struct_mutex); -- 1.8.1
[PATCH 20/62] drm/vmwgfx: convert to idr_alloc()
Convert to the much saner new idr interface. Only compile tested. Signed-off-by: Tejun Heo Cc: David Airlie Cc: dri-devel at lists.freedesktop.org --- This patch depends on an earlier idr changes and I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index e01a17b..c9d0676 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -177,17 +177,16 @@ int vmw_resource_alloc_id(struct vmw_resource *res) BUG_ON(res->id != -1); - do { - if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0)) - return -ENOMEM; - - write_lock(&dev_priv->resource_lock); - ret = idr_get_new_above(idr, res, 1, &res->id); - write_unlock(&dev_priv->resource_lock); + idr_preload(GFP_KERNEL); + write_lock(&dev_priv->resource_lock); - } while (ret == -EAGAIN); + ret = idr_alloc(idr, res, 1, 0, GFP_NOWAIT); + if (ret >= 0) + res->id = ret; - return ret; + write_unlock(&dev_priv->resource_lock); + idr_preload_end(); + return ret < 0 ? ret : 0; } /** -- 1.8.1
[PATCH v3 00/22] PCI: Iterate pci host bridge instead of pci root bus
On Sun, Jan 27, 2013 at 12:23 PM, Yinghai Lu wrote: > Now we have pci_root_buses list, and there is lots of iteration with > list_of_each of it, that is not safe after we add pci root bus hotplug > support after booting stage. > > Add pci_get_next_host_bridge and use bus_find_device in driver core to > iterate host bridge and the same time get root bus. > > We replace searching root bus with searching host_bridge, > as host_bridge->bus is the root bus. > After those replacing, we even could kill pci_root_buses list. These are the problems I think you're fixing: 1) pci_find_next_bus() is not safe because even though it holds pci_bus_sem while walking the pci_root_buses list, it doesn't hold a reference on the bus it returns. The bus could be removed while the caller is using it. 2) "list_for_each_entry(bus, &pci_root_buses, node)" is not safe because hotplug might modify the pci_root_buses list. Replacing that with for_each_pci_host_bridge() solves that problem by using bus_find_device(), which is built on klists, which are designed for that problem. 3) pci_find_next_bus() claims to iterate through all known PCI buses, but in fact only iterates through root buses. So far, so good. Those are problems we need to fix. Your solution is to introduce for_each_pci_host_bridge() as an iterator through the known host bridges. There are two scenarios where we use something like this: 1) We want to perform an operation on every known host bridge. 2) We want to initialize something for every host bridge. In my opinion, the only instance of scenario 1) is bus_rescan_store(), where we want to rescan all PCI host bridges. In every other case, we're doing some kind of initialization of all the host bridges. For these cases, for_each_pci_host_bridge() is the wrong solution because it doesn't work for hot-added bridges. I think these cases should be changed to use pcibios_root_bridge_prepare() or something something else called in the host bridge add path. Bjorn