* Reparenting a platform device @ 2012-04-05 8:42 Thierry Reding [not found] ` <20120405084258.GA19798-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Thierry Reding @ 2012-04-05 8:42 UTC (permalink / raw) To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA [-- Attachment #1.1: Type: text/plain, Size: 2880 bytes --] Hi, I have a device tree where I have a GART device and a DRM device which uses the GART. The GART is implemented by an IOMMU driver (tegra-gart) and requires the user device to be a child of the GART device (it explicitly checks for this when the user device is attached). I've tried two alternatives to achieve this: create the GART device in the user driver's .probe() function and explicitly set the DRM device's parent to the resulting platform device like so: gart = platform_device_alloc(...); ... pdev->dev.parent = &gart->dev; The alternative is to use the device tree to look up the GART device node and resolve it to the corresponding struct device: gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0); gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node); Where match_of_node matches each struct device's .of_node field to the gart_node. Both of these variants seem to work, and the DRM device can be properly attached to the GART device. However, after the DRM driver's .probe() exits, I get the following error: [ 25.579261] ------------[ cut here ]------------ [ 25.583895] WARNING: at /home/thierry.reding/src/kernel/linux-ipmp.git/kernel/mutex-debug.c:78 debug_mutex_unlock+0x114/0x128() [ 25.595352] Modules linked in: tegra_drm(+) cfbfillrect cfbimgblt cfbcopyarea drm_kms_helper drm fb pwm_tegra pwm_bl backlight [ 25.606821] [<c0013c54>] (unwind_backtrace+0x0/0xf8) from [<c0027bcc>] (warn_slowpath_common+0x4c/0x64) [ 25.616207] [<c0027bcc>] (warn_slowpath_common+0x4c/0x64) from [<c0027c00>] (warn_slowpath_null+0x1c/0x24) [ 25.625853] [<c0027c00>] (warn_slowpath_null+0x1c/0x24) from [<c00667fc>] (debug_mutex_unlock+0x114/0x128) [ 25.635508] [<c00667fc>] (debug_mutex_unlock+0x114/0x128) from [<c03bee2c>] (__mutex_unlock_slowpath+0x84/0x140) [ 25.645680] [<c03bee2c>] (__mutex_unlock_slowpath+0x84/0x140) from [<c02149b4>] (__driver_attach+0x78/0x90) [ 25.655412] [<c02149b4>] (__driver_attach+0x78/0x90) from [<c0213128>] (bus_for_each_dev+0x50/0x7c) [ 25.664449] [<c0213128>] (bus_for_each_dev+0x50/0x7c) from [<c0214038>] (bus_add_driver+0x174/0x234) [ 25.673572] [<c0214038>] (bus_add_driver+0x174/0x234) from [<c0214e7c>] (driver_register+0x78/0x12c) [ 25.682696] [<c0214e7c>] (driver_register+0x78/0x12c) from [<c0008620>] (do_one_initcall+0x34/0x174) [ 25.691826] [<c0008620>] (do_one_initcall+0x34/0x174) from [<c006dab0>] (sys_init_module+0xc20/0x18e0) [ 25.701131] [<c006dab0>] (sys_init_module+0xc20/0x18e0) from [<c000df00>] (ret_fast_syscall+0x0/0x30) [ 25.710336] ---[ end trace f64968a4a9d9fe8b ]--- I may be misinterpreting things, but this looks to be caused by the reparenting. So the question is whether what I'm trying to do is valid or if there is some other way to properly make the GART device the parent of the DRM device. Thanks, Thierry [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 192 bytes --] _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20120405084258.GA19798-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <20120405084258.GA19798-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> @ 2012-04-05 15:40 ` Stephen Warren [not found] ` <4F7DBCEE.9080400-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2012-04-07 2:24 ` Grant Likely 1 sibling, 1 reply; 13+ messages in thread From: Stephen Warren @ 2012-04-05 15:40 UTC (permalink / raw) To: Thierry Reding Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Hiroshi Doyu On 04/05/2012 02:42 AM, Thierry Reding wrote: > Hi, > > I have a device tree where I have a GART device and a DRM device which uses > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and > requires the user device to be a child of the GART device (it explicitly > checks for this when the user device is attached). Isn't this wrong? I would expect the device parent/child relationship to reflect the CPU-initiated register access bus topology. A device's interaction with an IOMMU is an aspect of a device's initiating accesses itself, not CPU-initiated register accesses. > I've tried two alternatives to achieve this: create the GART device in the > user driver's .probe() function and explicitly set the DRM device's parent > to the resulting platform device like so: > > gart = platform_device_alloc(...); > ... > pdev->dev.parent = &gart->dev; I guess that won't work when there's more than one device affected by the IOMMU? > The alternative is to use the device tree to look up the GART device node and > resolve it to the corresponding struct device: > > gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0); That seems more logical to me. > gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node); That part should probably be encapsulated into the IOMMU subsystem? In fact, even the of_parse_phandle should perhaps be hidden inside some IOMMU iommu_get() call, that can use DT as a data source, or some other data structure set up by board files. ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <4F7DBCEE.9080400-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <4F7DBCEE.9080400-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2012-04-05 16:23 ` Thierry Reding [not found] ` <20120405162318.GA10628-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Thierry Reding @ 2012-04-05 16:23 UTC (permalink / raw) To: Stephen Warren Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA [-- Attachment #1.1: Type: text/plain, Size: 2979 bytes --] * Stephen Warren wrote: > On 04/05/2012 02:42 AM, Thierry Reding wrote: > > Hi, > > > > I have a device tree where I have a GART device and a DRM device which uses > > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and > > requires the user device to be a child of the GART device (it explicitly > > checks for this when the user device is attached). > > Isn't this wrong? > > I would expect the device parent/child relationship to reflect the > CPU-initiated register access bus topology. > > A device's interaction with an IOMMU is an aspect of a device's > initiating accesses itself, not CPU-initiated register accesses. Actually I have no idea why this was made a requirement. Maybe Hiroshi can comment on this. The driver really only needs this to basically obtain a pointer to itself. The MSM I/O MMU implementation does something similar, though, and goes on to register actual child devices (they are instantiated in arch/arm/mach-msm/devices-iommu.c). Each of those devices is then assigned a specific memory area it seems. > > I've tried two alternatives to achieve this: create the GART device in the > > user driver's .probe() function and explicitly set the DRM device's parent > > to the resulting platform device like so: > > > > gart = platform_device_alloc(...); > > ... > > pdev->dev.parent = &gart->dev; > > I guess that won't work when there's more than one device affected by > the IOMMU? I don't think having more than one device using the IOMMU will work properly anyway in the context of the Tegra GART driver because there is not means to allocate specific regions of the GART aperture to individual devices. So really the one and only client actually needs to manage the allocations from the GART aperture. I'm also not sure if it makes much sense to use the GART from anything other than the DRM driver. > > The alternative is to use the device tree to look up the GART device node and > > resolve it to the corresponding struct device: > > > > gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0); > > That seems more logical to me. > > > gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node); > > That part should probably be encapsulated into the IOMMU subsystem? In > fact, even the of_parse_phandle should perhaps be hidden inside some > IOMMU iommu_get() call, that can use DT as a data source, or some other > data structure set up by board files. Both of these methods don't work too well because they actually require a reordering of the device hierarchy at runtime. For some reason this leads to the mutex becoming confused and the warning that I posted. I came up with a third alternative that does the reparenting in board-dt-tegra20.c after the call to of_platform_populate() which works similar to the second version and gets rid of the warning. I'm still not at all happy with it, though. Thierry [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 190 bytes --] _______________________________________________ iommu mailing list iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20120405162318.GA10628-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <20120405162318.GA10628-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> @ 2012-04-05 18:08 ` Thierry Reding [not found] ` <20120405180805.GB11601-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-05 18:36 ` Stephen Warren 1 sibling, 1 reply; 13+ messages in thread From: Thierry Reding @ 2012-04-05 18:08 UTC (permalink / raw) To: Stephen Warren Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Hiroshi Doyu [-- Attachment #1.1: Type: text/plain, Size: 1942 bytes --] * Thierry Reding wrote: > * Stephen Warren wrote: > > On 04/05/2012 02:42 AM, Thierry Reding wrote: > > > Hi, > > > > > > I have a device tree where I have a GART device and a DRM device which uses > > > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and > > > requires the user device to be a child of the GART device (it explicitly > > > checks for this when the user device is attached). > > > > Isn't this wrong? > > > > I would expect the device parent/child relationship to reflect the > > CPU-initiated register access bus topology. > > > > A device's interaction with an IOMMU is an aspect of a device's > > initiating accesses itself, not CPU-initiated register accesses. > > Actually I have no idea why this was made a requirement. Maybe Hiroshi can > comment on this. The driver really only needs this to basically obtain a > pointer to itself. The MSM I/O MMU implementation does something similar, > though, and goes on to register actual child devices (they are instantiated > in arch/arm/mach-msm/devices-iommu.c). Each of those devices is then assigned > a specific memory area it seems. [...] > I don't think having more than one device using the IOMMU will work properly > anyway in the context of the Tegra GART driver because there is not means to > allocate specific regions of the GART aperture to individual devices. So > really the one and only client actually needs to manage the allocations from > the GART aperture. I've been thinking about this some more and it occurred to me that maybe it would be best to add an additional layer between the GART and the clients which could manage the allocations. Such a virtual device could actually be registered as a child of the GART device and allow individual drivers to request mappings from it so that there is a single instance handing them out and keep them consistent. How does that sound? Thierry [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 192 bytes --] _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20120405180805.GB11601-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <20120405180805.GB11601-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> @ 2012-04-05 19:21 ` Stephen Warren [not found] ` <4F7DF0B3.4050500-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Stephen Warren @ 2012-04-05 19:21 UTC (permalink / raw) To: Thierry Reding Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On 04/05/2012 12:08 PM, Thierry Reding wrote: > * Thierry Reding wrote: >> * Stephen Warren wrote: >>> On 04/05/2012 02:42 AM, Thierry Reding wrote: >>>> Hi, >>>> >>>> I have a device tree where I have a GART device and a DRM device which uses >>>> the GART. The GART is implemented by an IOMMU driver (tegra-gart) and >>>> requires the user device to be a child of the GART device (it explicitly >>>> checks for this when the user device is attached). >>> >>> Isn't this wrong? >>> >>> I would expect the device parent/child relationship to reflect the >>> CPU-initiated register access bus topology. >>> >>> A device's interaction with an IOMMU is an aspect of a device's >>> initiating accesses itself, not CPU-initiated register accesses. >> >> Actually I have no idea why this was made a requirement. Maybe Hiroshi can >> comment on this. The driver really only needs this to basically obtain a >> pointer to itself. The MSM I/O MMU implementation does something similar, >> though, and goes on to register actual child devices (they are instantiated >> in arch/arm/mach-msm/devices-iommu.c). Each of those devices is then assigned >> a specific memory area it seems. > [...] >> I don't think having more than one device using the IOMMU will work properly >> anyway in the context of the Tegra GART driver because there is not means to >> allocate specific regions of the GART aperture to individual devices. So >> really the one and only client actually needs to manage the allocations from >> the GART aperture. > > I've been thinking about this some more and it occurred to me that maybe it > would be best to add an additional layer between the GART and the clients > which could manage the allocations. Such a virtual device could actually be > registered as a child of the GART device and allow individual drivers to > request mappings from it so that there is a single instance handing them out > and keep them consistent. > > How does that sound? I must admit I'm not at all familiar with the IOMMU APIs, but isn't the IOMMU driver/subsystem itself what is managing all the allocations and handing them out to clients? And client drivers do things like asking for N pages of memory mapped into their aperture? If that is true, I'm not sure what the purpose is of the intermediate device you propose. Sorry for being somewhat clueless. ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <4F7DF0B3.4050500-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <4F7DF0B3.4050500-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2012-04-06 7:20 ` Thierry Reding [not found] ` <20120406072014.GA14250-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Thierry Reding @ 2012-04-06 7:20 UTC (permalink / raw) To: Stephen Warren Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA [-- Attachment #1.1: Type: text/plain, Size: 1346 bytes --] * Stephen Warren wrote: > I must admit I'm not at all familiar with the IOMMU APIs, but isn't the > IOMMU driver/subsystem itself what is managing all the allocations and > handing them out to clients? And client drivers do things like asking > for N pages of memory mapped into their aperture? If that is true, I'm > not sure what the purpose is of the intermediate device you propose. > Sorry for being somewhat clueless. That was my impression too at first. But it seems like all the IOMMU subsystem does is map individual pages. So you pass it a physical address of a page along with the IO virtual address to map it to. That's at least the way it is handled for Tegra 2 GART (and from a quick look also by the Tegra 3 SMMU). For the SMMU the situation may be different because it may not have a fixed aperture that needs to contain the IO virtual addresses, though I must admit I haven't looked at Tegra 3 in enough detail to judge this. So this intermediate device would be purely an allocator for the GART aperture and handle the actual mapping via the IOMMU. This would probably be really simple and is in fact now done in the DRM driver. The nice thing if this would be a separate device is that it would be easy to make it a child of the IOMMU and wouldn't be as counter-intuitive as making the DRM device the IOMMU's child. Thierry [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 190 bytes --] _______________________________________________ iommu mailing list iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20120406072014.GA14250-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <20120406072014.GA14250-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org> @ 2012-04-06 16:37 ` Stephen Warren [not found] ` <4F7F1BCB.7070608-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Stephen Warren @ 2012-04-06 16:37 UTC (permalink / raw) To: Thierry Reding Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On 04/06/2012 01:20 AM, Thierry Reding wrote: > * Stephen Warren wrote: >> I must admit I'm not at all familiar with the IOMMU APIs, but isn't the >> IOMMU driver/subsystem itself what is managing all the allocations and >> handing them out to clients? And client drivers do things like asking >> for N pages of memory mapped into their aperture? If that is true, I'm >> not sure what the purpose is of the intermediate device you propose. >> Sorry for being somewhat clueless. > > That was my impression too at first. But it seems like all the IOMMU > subsystem does is map individual pages. So you pass it a physical address of > a page along with the IO virtual address to map it to. That's at least the > way it is handled for Tegra 2 GART (and from a quick look also by the Tegra 3 > SMMU). For the SMMU the situation may be different because it may not have a > fixed aperture that needs to contain the IO virtual addresses, though I must > admit I haven't looked at Tegra 3 in enough detail to judge this. > > So this intermediate device would be purely an allocator for the GART > aperture and handle the actual mapping via the IOMMU. This would probably be > really simple and is in fact now done in the DRM driver. The nice thing if > this would be a separate device is that it would be easy to make it a child > of the IOMMU and wouldn't be as counter-intuitive as making the DRM device > the IOMMU's child. OK, I see. I'll defer to Hiroshi here; he's far more familiar with all this than I am. My only comment is that I'd be surprised to see any kind of memory allocator represented as a driver (as opposed to a utility module) since it's not really representing an actual hardware block. ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <4F7F1BCB.7070608-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <4F7F1BCB.7070608-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> @ 2012-04-08 14:21 ` Thierry Reding [not found] ` <20120408142141.GA25303-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Thierry Reding @ 2012-04-08 14:21 UTC (permalink / raw) To: Stephen Warren Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA [-- Attachment #1.1: Type: text/plain, Size: 2063 bytes --] * Stephen Warren wrote: > On 04/06/2012 01:20 AM, Thierry Reding wrote: > > * Stephen Warren wrote: > >> I must admit I'm not at all familiar with the IOMMU APIs, but isn't the > >> IOMMU driver/subsystem itself what is managing all the allocations and > >> handing them out to clients? And client drivers do things like asking > >> for N pages of memory mapped into their aperture? If that is true, I'm > >> not sure what the purpose is of the intermediate device you propose. > >> Sorry for being somewhat clueless. > > > > That was my impression too at first. But it seems like all the IOMMU > > subsystem does is map individual pages. So you pass it a physical address of > > a page along with the IO virtual address to map it to. That's at least the > > way it is handled for Tegra 2 GART (and from a quick look also by the Tegra 3 > > SMMU). For the SMMU the situation may be different because it may not have a > > fixed aperture that needs to contain the IO virtual addresses, though I must > > admit I haven't looked at Tegra 3 in enough detail to judge this. > > > > So this intermediate device would be purely an allocator for the GART > > aperture and handle the actual mapping via the IOMMU. This would probably be > > really simple and is in fact now done in the DRM driver. The nice thing if > > this would be a separate device is that it would be easy to make it a child > > of the IOMMU and wouldn't be as counter-intuitive as making the DRM device > > the IOMMU's child. > > OK, I see. > > I'll defer to Hiroshi here; he's far more familiar with all this than I am. > > My only comment is that I'd be surprised to see any kind of memory > allocator represented as a driver (as opposed to a utility module) since > it's not really representing an actual hardware block. I agree. In that case I don't see any other solution than to remove the requirement of the parent/child relationship from the GART driver. It seems like the SMMU driver doesn't have such a requirement so it should be fine. Thierry [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 190 bytes --] _______________________________________________ iommu mailing list iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20120408142141.GA25303-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <20120408142141.GA25303-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> @ 2012-04-10 8:25 ` Hiroshi Doyu 0 siblings, 0 replies; 13+ messages in thread From: Hiroshi Doyu @ 2012-04-10 8:25 UTC (permalink / raw) To: thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, swarren-3lzwWm7+Weoh9ZMKESR00Q From: Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> Subject: Re: Reparenting a platform device Date: Sun, 8 Apr 2012 16:21:41 +0200 Message-ID: <20120408142141.GA25303-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> > * PGP Signed by an unknown key > > * Stephen Warren wrote: > > On 04/06/2012 01:20 AM, Thierry Reding wrote: > > > * Stephen Warren wrote: > > >> I must admit I'm not at all familiar with the IOMMU APIs, but isn't the > > >> IOMMU driver/subsystem itself what is managing all the allocations and > > >> handing them out to clients? And client drivers do things like asking > > >> for N pages of memory mapped into their aperture? If that is true, I'm > > >> not sure what the purpose is of the intermediate device you propose. > > >> Sorry for being somewhat clueless. > > > > > > That was my impression too at first. But it seems like all the IOMMU > > > subsystem does is map individual pages. So you pass it a physical address of > > > a page along with the IO virtual address to map it to. That's at least the > > > way it is handled for Tegra 2 GART (and from a quick look also by the Tegra 3 > > > SMMU). For the SMMU the situation may be different because it may not have a > > > fixed aperture that needs to contain the IO virtual addresses, though I must > > > admit I haven't looked at Tegra 3 in enough detail to judge this. > > > > > > So this intermediate device would be purely an allocator for the GART > > > aperture and handle the actual mapping via the IOMMU. This would probably be > > > really simple and is in fact now done in the DRM driver. The nice thing if > > > this would be a separate device is that it would be easy to make it a child > > > of the IOMMU and wouldn't be as counter-intuitive as making the DRM device > > > the IOMMU's child. > > > > OK, I see. > > > > I'll defer to Hiroshi here; he's far more familiar with all this than I am. > > > > My only comment is that I'd be surprised to see any kind of memory > > allocator represented as a driver (as opposed to a utility module) since > > it's not really representing an actual hardware block. > > I agree. In that case I don't see any other solution than to remove the > requirement of the parent/child relationship from the GART driver. It seems > like the SMMU driver doesn't have such a requirement so it should be fine. Right. I missed one patch to send out. Probably we need the following patch for the above. >From f7a56c85a5dfe41d23746d28b2ecbfb66ef034b5 Mon Sep 17 00:00:00 2001 From: Vandana Salve <vsalve-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Date: Wed, 14 Mar 2012 18:17:53 +0530 Subject: [PATCH 1/1] iommu: tegra/gart: use correct gart_device Pass the correct gart device pointer. Reviewed-by: Vandana Salve <vsalve-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Tested-by: Vandana Salve <vsalve-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Reviewed-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Reviewed-by: Bharat Nihalani <bnihalani-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> Signed-off-by: Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- drivers/iommu/tegra-gart.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c index e8d50a2..c33557c 100644 --- a/drivers/iommu/tegra-gart.c +++ b/drivers/iommu/tegra-gart.c @@ -158,7 +158,7 @@ static int gart_iommu_attach_dev(struct iommu_domain *domain, struct gart_client *client, *c; int err = 0; - gart = dev_get_drvdata(dev->parent); + gart = gart_handle; if (!gart) return -EINVAL; domain->priv = gart; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: Reparenting a platform device [not found] ` <20120405162318.GA10628-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-05 18:08 ` Thierry Reding @ 2012-04-05 18:36 ` Stephen Warren 1 sibling, 0 replies; 13+ messages in thread From: Stephen Warren @ 2012-04-05 18:36 UTC (permalink / raw) To: Thierry Reding Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On 04/05/2012 10:23 AM, Thierry Reding wrote: > * Stephen Warren wrote: >> On 04/05/2012 02:42 AM, Thierry Reding wrote: >>> Hi, >>> >>> I have a device tree where I have a GART device and a DRM device which uses >>> the GART. The GART is implemented by an IOMMU driver (tegra-gart) and >>> requires the user device to be a child of the GART device (it explicitly >>> checks for this when the user device is attached). >> >> Isn't this wrong? >> >> I would expect the device parent/child relationship to reflect the >> CPU-initiated register access bus topology. >> >> A device's interaction with an IOMMU is an aspect of a device's >> initiating accesses itself, not CPU-initiated register accesses. > > Actually I have no idea why this was made a requirement. Maybe Hiroshi can > comment on this. The driver really only needs this to basically obtain a > pointer to itself. The MSM I/O MMU implementation does something similar, > though, and goes on to register actual child devices (they are instantiated > in arch/arm/mach-msm/devices-iommu.c). Each of those devices is then assigned > a specific memory area it seems. > >>> I've tried two alternatives to achieve this: create the GART device in the >>> user driver's .probe() function and explicitly set the DRM device's parent >>> to the resulting platform device like so: >>> >>> gart = platform_device_alloc(...); >>> ... >>> pdev->dev.parent = &gart->dev; >> >> I guess that won't work when there's more than one device affected by >> the IOMMU? > > I don't think having more than one device using the IOMMU will work properly > anyway in the context of the Tegra GART driver because there is not means to > allocate specific regions of the GART aperture to individual devices. So > really the one and only client actually needs to manage the allocations from > the GART aperture. > > I'm also not sure if it makes much sense to use the GART from anything other > than the DRM driver. I was thinking about this mostly from a Tegra30 perspective, where there are multiple devices affected by the SMMU, which is more capable that the Tegra20 GART, and can presumably support these multiple clients pretty independently. For Tegra20, I wouldn't be surprised if the DRM driver was the only client. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Reparenting a platform device [not found] ` <20120405084258.GA19798-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-05 15:40 ` Stephen Warren @ 2012-04-07 2:24 ` Grant Likely 2012-04-07 11:35 ` Thierry Reding 1 sibling, 1 reply; 13+ messages in thread From: Grant Likely @ 2012-04-07 2:24 UTC (permalink / raw) To: Thierry Reding, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On Thu, 5 Apr 2012 10:42:58 +0200, Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> wrote: > Hi, > > I have a device tree where I have a GART device and a DRM device which uses > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and > requires the user device to be a child of the GART device (it explicitly > checks for this when the user device is attached). > > I've tried two alternatives to achieve this: create the GART device in the > user driver's .probe() function and explicitly set the DRM device's parent > to the resulting platform device like so: > > gart = platform_device_alloc(...); > ... > pdev->dev.parent = &gart->dev; Yeah, don't *ever* try to do this. The device hierarchy is a complex data structure which must never be directly manipulated. > > The alternative is to use the device tree to look up the GART device node and > resolve it to the corresponding struct device: > > gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0); > gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node); > > Where match_of_node matches each struct device's .of_node field to the > gart_node. > > Both of these variants seem to work, and the DRM device can be properly > attached to the GART device. However, after the DRM driver's .probe() exits, > I get the following error: I don't understand what you're trying to describe here as the 2nd option. Regardless, reparenting should not ben the solution at all. What does the device tree that you envision look like for this? What devices are created, and what drivers bind to them? g. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Reparenting a platform device 2012-04-07 2:24 ` Grant Likely @ 2012-04-07 11:35 ` Thierry Reding [not found] ` <20120407113510.GA22116-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org> 0 siblings, 1 reply; 13+ messages in thread From: Thierry Reding @ 2012-04-07 11:35 UTC (permalink / raw) To: Grant Likely Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA [-- Attachment #1.1: Type: text/plain, Size: 2803 bytes --] * Grant Likely wrote: > On Thu, 5 Apr 2012 10:42:58 +0200, Thierry Reding <thierry.reding@avionic-design.de> wrote: > > Hi, > > > > I have a device tree where I have a GART device and a DRM device which uses > > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and > > requires the user device to be a child of the GART device (it explicitly > > checks for this when the user device is attached). > > > > I've tried two alternatives to achieve this: create the GART device in the > > user driver's .probe() function and explicitly set the DRM device's parent > > to the resulting platform device like so: > > > > gart = platform_device_alloc(...); > > ... > > pdev->dev.parent = &gart->dev; > > Yeah, don't *ever* try to do this. The device hierarchy is a complex > data structure which must never be directly manipulated. > > > > > The alternative is to use the device tree to look up the GART device node and > > resolve it to the corresponding struct device: > > > > gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0); > > gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node); > > > > Where match_of_node matches each struct device's .of_node field to the > > gart_node. > > > > Both of these variants seem to work, and the DRM device can be properly > > attached to the GART device. However, after the DRM driver's .probe() exits, > > I get the following error: > > I don't understand what you're trying to describe here as the 2nd > option. > > Regardless, reparenting should not ben the solution at all. What does > the device tree that you envision look like for this? What devices > are created, and what drivers bind to them? The reason why I need to reparent at all is because the IOMMU driver requires the user to be a child of the IOMMU device. If you look at the driver in drivers/iommu/tegra-gart.c you'll see that it references dev->parent in several places, most notably in the gart_iommu_attach_dev() function. So there's really only two options that I can see: 1) create a virtual device that is a child of the GART and is in charge of the actual allocations from the GART and have the DRM driver use that interface or 2) change the GART driver's behaviour in a way that the parent/child relationship is no longer a requirement. 1) has the advantage of providing a central allocation manager for the GART and will allow to register multiple clients with the GART. 2) does not have that advantage. Another alternative may be to allow only a single device to attach to the GART that doesn't have to be a child of the GART. That way the DRM could take care of GART aperture allocations, which seems to be the most logical place to do that anyway. Thierry [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --] [-- Attachment #2: Type: text/plain, Size: 190 bytes --] _______________________________________________ iommu mailing list iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20120407113510.GA22116-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>]
* Re: Reparenting a platform device [not found] ` <20120407113510.GA22116-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org> @ 2012-04-20 2:26 ` Grant Likely 0 siblings, 0 replies; 13+ messages in thread From: Grant Likely @ 2012-04-20 2:26 UTC (permalink / raw) To: Thierry Reding Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On Sat, 7 Apr 2012 13:35:10 +0200, Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> wrote: > * Grant Likely wrote: > > On Thu, 5 Apr 2012 10:42:58 +0200, Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org> wrote: > > > Hi, > > > > > > I have a device tree where I have a GART device and a DRM device which uses > > > the GART. The GART is implemented by an IOMMU driver (tegra-gart) and > > > requires the user device to be a child of the GART device (it explicitly > > > checks for this when the user device is attached). > > > > > > I've tried two alternatives to achieve this: create the GART device in the > > > user driver's .probe() function and explicitly set the DRM device's parent > > > to the resulting platform device like so: > > > > > > gart = platform_device_alloc(...); > > > ... > > > pdev->dev.parent = &gart->dev; > > > > Yeah, don't *ever* try to do this. The device hierarchy is a complex > > data structure which must never be directly manipulated. > > > > > > > > The alternative is to use the device tree to look up the GART device node and > > > resolve it to the corresponding struct device: > > > > > > gart_node = of_parse_phandle(drm->dev->of_node, "gart-parent", 0); > > > gart = bus_find_device(drm->dev->bus, NULL, gart_node, match_of_node); > > > > > > Where match_of_node matches each struct device's .of_node field to the > > > gart_node. > > > > > > Both of these variants seem to work, and the DRM device can be properly > > > attached to the GART device. However, after the DRM driver's .probe() exits, > > > I get the following error: > > > > I don't understand what you're trying to describe here as the 2nd > > option. > > > > Regardless, reparenting should not ben the solution at all. What does > > the device tree that you envision look like for this? What devices > > are created, and what drivers bind to them? > > The reason why I need to reparent at all is because the IOMMU driver requires > the user to be a child of the IOMMU device. If you look at the driver in > drivers/iommu/tegra-gart.c you'll see that it references dev->parent in > several places, most notably in the gart_iommu_attach_dev() function. So > there's really only two options that I can see: 1) create a virtual device > that is a child of the GART and is in charge of the actual allocations from > the GART and have the DRM driver use that interface or 2) change the GART > driver's behaviour in a way that the parent/child relationship is no longer a > requirement. Either is fine by me. > 1) has the advantage of providing a central allocation manager for the GART > and will allow to register multiple clients with the GART. 2) does not have > that advantage. > > Another alternative may be to allow only a single device to attach to the > GART that doesn't have to be a child of the GART. That way the DRM could take > care of GART aperture allocations, which seems to be the most logical place > to do that anyway. That also works. As long as nothing messes about with odd reparenting then I'm happy. g. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-04-20 2:26 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-05 8:42 Reparenting a platform device Thierry Reding [not found] ` <20120405084258.GA19798-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-05 15:40 ` Stephen Warren [not found] ` <4F7DBCEE.9080400-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2012-04-05 16:23 ` Thierry Reding [not found] ` <20120405162318.GA10628-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-05 18:08 ` Thierry Reding [not found] ` <20120405180805.GB11601-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-05 19:21 ` Stephen Warren [not found] ` <4F7DF0B3.4050500-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2012-04-06 7:20 ` Thierry Reding [not found] ` <20120406072014.GA14250-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org> 2012-04-06 16:37 ` Stephen Warren [not found] ` <4F7F1BCB.7070608-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2012-04-08 14:21 ` Thierry Reding [not found] ` <20120408142141.GA25303-RM9K5IK7kjIyiCvfTdI0JKcOhU4Rzj621B7CTYaBSLdn68oJJulU0Q@public.gmane.org> 2012-04-10 8:25 ` Hiroshi Doyu 2012-04-05 18:36 ` Stephen Warren 2012-04-07 2:24 ` Grant Likely 2012-04-07 11:35 ` Thierry Reding [not found] ` <20120407113510.GA22116-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org> 2012-04-20 2:26 ` Grant Likely
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.