Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/15/2012 02:39 AM, Thierry Reding wrote: * Stephen Warren wrote: On 04/13/2012 03:14 AM, Thierry Reding wrote: display-controllers = disp1 disp2; outputs = lvds hdmi tvo dsi; I don't think you need both the child nodes and those two properties. In other words, I think you either want: graphics@5400 { ... a bunch of child nodes }; or you want: disp1 : dc@5420 { ... }; disp2 : dc@5424 { ... }; ... all the other graphics nodes graphics@5400 { display-controllers = disp1 disp2; outputs = lvds hdmi tvo dsi; }; In the former case, presumably the drivers for the child nodes would make some API call into the parent node and just register themselves directly as a certain type of driver, so avoiding the display-controllers/outputs properties. I think I like the former better. The way I understand it the children of the graphics node will have to be registered explicitly by the DRM driver because of_platform_populate() doesn't work recursively. That would ensure that the DRM driver can setup the CRTC and output registries before the other devices call back into the DRM to register themselves. Yes, with the first way, the DRM driver will have to call of_platform_populate() recursively to make this work. The thing here is that the device tree should model hardware, not be designed purely to match the device registration needs of the DRM driver. I'm not sure that it's correct to model all those devices as children of the top-level graphics object; I /think/ all the devices are flat on a single bus, and hence not children of each-other. That all said, I guess having the nodes as children isn't too far off how the HW is designed (even if the register accesses aren't on a child bus, the modules at least logically are grouped together in an umbrella situation), so I wouldn't push back on the first option above that you prefer. /* initial configuration */ configuration { lvds { display-controller = disp1; output = lvds; }; hdmi { display-controller = disp2; output = hdmi; }; }; }; I added an additional node for the initial configuration so that the driver knows which mapping to setup at boot. Isn't that kind of thing usually set up by the video= KMS-related kernel command-line option? See Documentation/fb/modedb.txt. Again here, I think the actual display controllers would be allocated to whichever outputs get used on a first-come first-serve based, so no need for the display-controller property above either way. Boards should still be able to boot and display a console on the standard output even if the user doesn't provide a video= option. Shouldn't there be a way for a board DTS to specify what the default (or even allowed) connections are? Why wouldn't the default be to light up all outputs that have an attached display, or an algorithm something like: * If internal LCD is present, use that * Else, if HDMI display plugged in, use that ... Evaluation hardware like the Harmony might have LVDS, HDMI and VGA connectors to provide for a wide range of use cases. The Plutux for instance has only an HDMI connector and the Medcom has only LVDS. For the Medcom it would be quite confusing for people to suddenly see an HDMI-1 connector pop up f.e. in xrandr. It would be equally useless for the Plutux to show up as supporting an LVDS or VGA connector. So the device tree for those devices would disable (or not include) the connectors that were not present on the board. ... I see. Maybe this could be used for board-specific configuration? For example, the Plutux could have something like this: connectors { hdmi { output = hdmi; ddc = i2c2; }; }; The Medcom could have: connectors { lvds { output = lvds; edid = edid; }; }; While Harmony could be more generic and provide more outputs: connectors { lvds { output = lvds; ddc = i2c1; }; vga { /* which output is used for VGA? */ output = ...; ddc = i2c2; hdmi { output = hdmi; ddc = i2c3; }; }; That looks like a reasonable start. Has there been any discussion as to how EDID data would best be represented in DT? Should it just be a binary blob or rather some textual representation? I think
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Stephen Warren wrote: On 04/15/2012 02:39 AM, Thierry Reding wrote: I think I like the former better. The way I understand it the children of the graphics node will have to be registered explicitly by the DRM driver because of_platform_populate() doesn't work recursively. That would ensure that the DRM driver can setup the CRTC and output registries before the other devices call back into the DRM to register themselves. Yes, with the first way, the DRM driver will have to call of_platform_populate() recursively to make this work. The thing here is that the device tree should model hardware, not be designed purely to match the device registration needs of the DRM driver. I'm not sure that it's correct to model all those devices as children of the top-level graphics object; I /think/ all the devices are flat on a single bus, and hence not children of each-other. That all said, I guess having the nodes as children isn't too far off how the HW is designed (even if the register accesses aren't on a child bus, the modules at least logically are grouped together in an umbrella situation), so I wouldn't push back on the first option above that you prefer. After trying to implement this I'm not so sure anymore that this is the best approach. I think I'll have to play around with this some more to see what fits best. Boards should still be able to boot and display a console on the standard output even if the user doesn't provide a video= option. Shouldn't there be a way for a board DTS to specify what the default (or even allowed) connections are? Why wouldn't the default be to light up all outputs that have an attached display, or an algorithm something like: * If internal LCD is present, use that * Else, if HDMI display plugged in, use that ... That sounds doable. Evaluation hardware like the Harmony might have LVDS, HDMI and VGA connectors to provide for a wide range of use cases. The Plutux for instance has only an HDMI connector and the Medcom has only LVDS. For the Medcom it would be quite confusing for people to suddenly see an HDMI-1 connector pop up f.e. in xrandr. It would be equally useless for the Plutux to show up as supporting an LVDS or VGA connector. So the device tree for those devices would disable (or not include) the connectors that were not present on the board. Okay, makes sense. Has there been any discussion as to how EDID data would best be represented in DT? Should it just be a binary blob or rather some textual representation? I think a binary blob makes sense - that's the exact same format it'd have if read over the DDC I2C bus. DTC has /incbin/ for that. Is arch/arm/boot/dts still the correct place for EDID blobs? I could add tegra-medcom.edid if that's okay. Thierry pgpH0jdIY00Rx.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/16/2012 12:48 PM, Thierry Reding wrote: * Stephen Warren wrote: ... Has there been any discussion as to how EDID data would best be represented in DT? Should it just be a binary blob or rather some textual representation? I think a binary blob makes sense - that's the exact same format it'd have if read over the DDC I2C bus. DTC has /incbin/ for that. Is arch/arm/boot/dts still the correct place for EDID blobs? I could add tegra-medcom.edid if that's okay. As far as I know, yes. Perhaps we'll want to start putting stuff in SoC-specific sub-directories given the number of files we'll end up with here (irrespective of EDID etc.), but I haven't seen any move towards that yet. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Stephen Warren wrote: On 04/16/2012 12:48 PM, Thierry Reding wrote: * Stephen Warren wrote: ... Has there been any discussion as to how EDID data would best be represented in DT? Should it just be a binary blob or rather some textual representation? I think a binary blob makes sense - that's the exact same format it'd have if read over the DDC I2C bus. DTC has /incbin/ for that. Is arch/arm/boot/dts still the correct place for EDID blobs? I could add tegra-medcom.edid if that's okay. As far as I know, yes. Perhaps we'll want to start putting stuff in SoC-specific sub-directories given the number of files we'll end up with here (irrespective of EDID etc.), but I haven't seen any move towards that yet. Yes, especially as more machines are moving to DT that directory will soon become quite cluttered. I suppose a tegra subdirectory wouldn't hurt. I've been looking about for tools to generate EDID data but didn't find anything useful. Does anyone know of any tool that's more convenient than manually filling a struct edid and writing that to a file? Thierry pgpfhYZwO5Nw5.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/16/2012 01:03 PM, Thierry Reding wrote: ... I've been looking about for tools to generate EDID data but didn't find anything useful. Does anyone know of any tool that's more convenient than manually filling a struct edid and writing that to a file? Sorry, no. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Stephen Warren wrote: On 04/13/2012 03:14 AM, Thierry Reding wrote: display-controllers = disp1 disp2; outputs = lvds hdmi tvo dsi; I don't think you need both the child nodes and those two properties. In other words, I think you either want: graphics@5400 { ... a bunch of child nodes }; or you want: disp1 : dc@5420 { ... }; disp2 : dc@5424 { ... }; ... all the other graphics nodes graphics@5400 { display-controllers = disp1 disp2; outputs = lvds hdmi tvo dsi; }; In the former case, presumably the drivers for the child nodes would make some API call into the parent node and just register themselves directly as a certain type of driver, so avoiding the display-controllers/outputs properties. I think I like the former better. The way I understand it the children of the graphics node will have to be registered explicitly by the DRM driver because of_platform_populate() doesn't work recursively. That would ensure that the DRM driver can setup the CRTC and output registries before the other devices call back into the DRM to register themselves. /* initial configuration */ configuration { lvds { display-controller = disp1; output = lvds; }; hdmi { display-controller = disp2; output = hdmi; }; }; }; I added an additional node for the initial configuration so that the driver knows which mapping to setup at boot. Isn't that kind of thing usually set up by the video= KMS-related kernel command-line option? See Documentation/fb/modedb.txt. Again here, I think the actual display controllers would be allocated to whichever outputs get used on a first-come first-serve based, so no need for the display-controller property above either way. Boards should still be able to boot and display a console on the standard output even if the user doesn't provide a video= option. Shouldn't there be a way for a board DTS to specify what the default (or even allowed) connections are? Evaluation hardware like the Harmony might have LVDS, HDMI and VGA connectors to provide for a wide range of use cases. The Plutux for instance has only an HDMI connector and the Medcom has only LVDS. For the Medcom it would be quite confusing for people to suddenly see an HDMI-1 connector pop up f.e. in xrandr. It would be equally useless for the Plutux to show up as supporting an LVDS or VGA connector. What I don't quite see yet is where to attach EDID data or pass the phandle to the I2C controller for DDC/EDID probing. I think there's a third node type. 1) Nodes for the display controllers in Tegra (Also known as CRTCs I believe) 2) Nodes for the video outputs from the Tegra chips (what you have as outputs above) 3) Connectors, which aren't represented in your DT above yet. I think you need to add additional nodes for (3). These are where you'd represent all the EDID/I2C/... stuff. Then, the nodes for the outputs will point at the nodes for the connectors using phandles, or vice-versa. I see. Maybe this could be used for board-specific configuration? For example, the Plutux could have something like this: connectors { hdmi { output = hdmi; ddc = i2c2; }; }; The Medcom could have: connectors { lvds { output = lvds; edid = edid; }; }; While Harmony could be more generic and provide more outputs: connectors { lvds { output = lvds; ddc = i2c1; }; vga { /* which output is used for VGA? */ output = ...; ddc = i2c2; hdmi { output = hdmi; ddc = i2c3; }; }; Has there been any discussion as to how EDID data would best be represented in DT? Should it just be a binary blob or rather some textual representation? (IIRC, although I didn't look at them in detail, the sdrm patches recently mentioned something about splitting up the various object types in DRM and allowing them to be represented separately, and I vaguely recall something along the lines of the types I mention above. I might expect to have a 1:1 mapping between the DRM object types and DT nodes.) I've looked at them in more detail and I'm not sure that they're very well suited for the Tegra. I think they may be a bit too simple to support
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Stephen Warren wrote: On 04/12/2012 11:44 AM, Thierry Reding wrote: [...] And given that, I don't think we should name the node after some OS-specific software concept. Device tree is intended to model hardware. [...] Maybe one solution would be to have a top-level DRM device with a register map from 0x5400 to 0x547f, which the TRM designates as host registers. Then subnodes could be used for the subdevices. Ah yes, just what I was thinking above:-) I came up with the following: /* host1x */ host1x : host1x@5000 { reg = 0x5000 0x00024000; interrupts = 0 64 0x04 /* cop syncpt */ 0 65 0x04 /* mpcore syncpt */ 0 66 0x04 /* cop general */ 0 67 0x04; /* mpcore general */ }; /* graphics host */ graphics@5400 { compatible = nvidia,tegra20-graphics; #address-cells = 1; #size-cells = 1; ranges = 0 0x5400 0x0800; host1x = host1x; /* video-encoding/decoding */ mpe@5404 { reg = 0x5404 0x0004; interrupts = 0 68 0x04; }; /* video input */ vi@5408 { reg = 0x5408 0x0004; interrupts = 0 69 0x04; }; /* EPP */ epp@540c { reg = 0x540c 0x0004; interrupts = 0 70 0x04; } /* ISP */ isp@5410 { reg = 0x5410 0x0004; interrupts = 0 71 0x04; }; /* 2D engine */ gr2d@5414 { reg = 0x5414 0x0004; interrupts = 0 72 0x04; }; /* 3D engine */ gr3d@5418 { reg = 0x5418 0x0004; }; /* display controllers */ disp1 : dc@5420 { compatible = nvidia,tegra20-dc; reg = 0x5420 0x0004; interrupts = 0 73 0x04; }; disp2 : dc@5424 { compatible = nvidia,tegra20-dc; reg = 0x5424 0x0004; interrupts = 0 74 0x04; }; /* outputs */ lvds : rgb { compatible = nvidia,tegra20-rgb; }; hdmi : hdmi@5428 { compatible = nvidia,tegra20-hdmi; reg = 0x5428 0x0004; interrupts = 0 75 0x04; }; tvo : tvo@542c { compatible = nvidia,tegra20-tvo; reg = 0x542c 0x0004; interrupts = 0 76 0x04; }; dsi : dsi@5430 { compatible = nvidia,tegra20-dsi; reg = 0x5430 0x0004; }; display-controllers = disp1 disp2; outputs = lvds hdmi tvo dsi; /* initial configuration */ configuration { lvds { display-controller = disp1; output = lvds; }; hdmi { display-controller = disp2; output = hdmi; }; }; }; I added an additional node for the initial configuration so that the driver knows which mapping to setup at boot. What I don't quite see yet is where to attach EDID data or pass the phandle to the I2C controller for DDC/EDID probing. The initial configuration is certainly not the right place. Perhaps the outputs property should be made a node instead: outputs { lvds_out { output = lvds; edid = edid; }; hdmi_out { output = hdmi; ddc = i2c2; }; }; But then outputs should probably become something like connectors instead and the initial configuration refers to the _out phandles. Thierry pgpTKMG5emx8V.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Arnd Bergmann wrote: On Wednesday 11 April 2012, Thierry Reding wrote: Daniel Vetter wrote: Well, you use the iommu api to map/unmap memory into the iommu for tegra, whereas usually device drivers just use the dma api to do that. The usual interface is dma_map_sg/dma_unmap_sg, but there are quite a few variants around. I'm just wondering why this you've choosen this. I don't think this works on ARM. Maybe I'm not seeing the whole picture but judging by a quick look through the kernel tree there aren't any users that map DMA memory through an IOMMU. dma_map_sg is certainly the right interface to use, and Marek Szyprowski has patches to make that work on ARM, hopefully going into v3.5, so you could use those. I've looked at Marek's patches but I don't think they'll work for Tegra 2 or Tegra 3. The corresponding iommu_map() functions only set one PTE, regardless of the number of bytes passed to them. However, the Tegra TRM indicates that mapping needs to be done on a per-page basis so contiguous regions cannot be combined. I suppose the IOMMU driver would have to be fixed to program more than a single page in that case. Also this doesn't yet solve the vmap() problem that is needed for the kernel virtual mapping. I did try using dma_alloc_writecombine(), but that only works for chunks of 2 MB or smaller, unless I use init_consistent_dma_size() during board setup, which isn't provided for in a DT setup. I couldn't find a better alternative, but I admit I'm not very familiar with all the VM APIs. Do you have any suggestions on how to solve this? Otherwise I'll try and dig in some more. Thierry pgpvwmfXfBSxK.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Stephen Warren wrote: On 04/11/2012 06:10 AM, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. I'll let Jon Mayo comment on the actual driver implementation, since he's a lot more familiar with Tegra's display hardware. However, I have some general comments below. .../devicetree/bindings/gpu/drm/tegra.txt | 24 + arch/arm/mach-tegra/board-dt-tegra20.c |3 + arch/arm/mach-tegra/tegra2_clocks.c|8 +- drivers/gpu/drm/Kconfig|2 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/tegra/Kconfig | 10 + drivers/gpu/drm/tegra/Makefile |5 + drivers/gpu/drm/tegra/tegra_drv.c | 2241 drivers/gpu/drm/tegra/tegra_drv.h | 184 ++ include/drm/tegra_drm.h| 44 + Splitting this patch into two, between arch/arm and drivers/gpu would be a good idea. I can certainly do that. diff --git a/Documentation/devicetree/bindings/gpu/drm/tegra.txt b/Documentation/devicetree/bindings/gpu/drm/tegra.txt + drm@5420 { + compatible = nvidia,tegra20-drm; This doesn't seem right; there isn't a DRM hardware module on Tegra, since DRM is a Linux/software-specific term. I'd at least expect to see this compatible flag be renamed to something more like nvidia,tegra20-dc (dc==display controller). Since Tegra has two display controller modules (I believe identical?), and numerous other independent(?) blocks, I'd expect to see multiple nodes in device tree, one per hardware block, such that each block gets its own device and driver. That said, I'm not familiar enough with Tegra's display and graphics HW to know if this makes sense. Jon, what's your take here? The clock change below, and in particular the original code there that we use downstream, lends weight to my argument. + reg = 0x5420 0x0004/* display A */ + 0x5424 0x0004/* display B */ + 0x5800 0x0200 ; /* GART aperture */ + interrupts = 0 73 0x04/* display A */ + 0 74 0x04 ; /* display B */ + + lvds { + type = rgb; These sub-nodes probably want a compatible property rather than a type property. compatible suggests that a driver would bind to it. However the data really is only passed to the DC driver for configuration. + size = 345 194; + + default-mode { + pixel-clock = 61715000; + vertical-refresh = 50; + resolution = 1366 768; + bits-per-pixel = 16; + horizontal-timings = 4 136 2 36; + vertical-timings = 2 4 21 10; + }; + }; I imagine that quite a bit of thought needs to be put into the output part of the binding in order to: * Model the outputs/connectors separately from display controllers. * Make sure that the basic infra-structure for representing an output is general enough to be extensible to all the kinds of outputs we support, not just the LVDS output. I haven't played around with HDMI at all yet, so I don't know of the requirements. I'm pretty sure the above isn't anywhere near complete though. * We were wondering about putting an EDID into the DT to represent the display modes, so that all outputs had EDIDs rather than real monitors having EDIDs, and fixed internal displays having some other representation of capabilities. That's an interesting approach. I like it. I'm hoping that Jon will drive this. diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c - PERIPH_CLK(disp1, tegradc.0,NULL, 27, 0x138, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ - PERIPH_CLK(disp2, tegradc.1,NULL, 26, 0x13c, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ + PERIPH_CLK(disp1, tegra-drm,NULL, 27, 0x138, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ + PERIPH_CLK(disp2, tegra-drm,NULL, 26, 0x13c, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ This doesn't seem right, and couples back to my assertion above that the two
RE: [RFC 4/4] drm: Add NVIDIA Tegra support
Hi Thierry, On Thursday, April 12, 2012 9:18 AM Thierry Reding wrote: * Arnd Bergmann wrote: On Wednesday 11 April 2012, Thierry Reding wrote: Daniel Vetter wrote: Well, you use the iommu api to map/unmap memory into the iommu for tegra, whereas usually device drivers just use the dma api to do that. The usual interface is dma_map_sg/dma_unmap_sg, but there are quite a few variants around. I'm just wondering why this you've choosen this. I don't think this works on ARM. Maybe I'm not seeing the whole picture but judging by a quick look through the kernel tree there aren't any users that map DMA memory through an IOMMU. dma_map_sg is certainly the right interface to use, and Marek Szyprowski has patches to make that work on ARM, hopefully going into v3.5, so you could use those. I've looked at Marek's patches but I don't think they'll work for Tegra 2 or Tegra 3. The corresponding iommu_map() functions only set one PTE, regardless of the number of bytes passed to them. However, the Tegra TRM indicates that mapping needs to be done on a per-page basis so contiguous regions cannot be combined. I suppose the IOMMU driver would have to be fixed to program more than a single page in that case. I assume you want to map a set of pages into contiguous chunk in io address space. This can be done with dma_map_sg() call once IOMMU aware implementation has been assigned to the given device. DMA-mapping implementation is able to merge consecutive chunks of the scatter list in the dma/io address space if possible (i.e. there are no in-page offsets between the chunks). With my implementation of IOMMU aware dma-mapping you usually you get a single DMA chunk from the provided scatter-list. I know that this approach causes a lot of confusion at the first look, but that how dma mapping api has been designed. The scatter list based approach has some drawbacks - it is a bit oversized for most of the typical use cases for the gfx/multimedia buffers, but that's all we have now. Scatter lists were initially designed for the disk based block io operations, hence the presence of the in-page offsets and lengths for each chunk. For multimedia use cases providing an array of struct pages and asking dma-mapping to map them into contiguous memory is probably all we need. I wonder if introducing such new calls is a good idea. Anrd, what do think? It will definitely simplify the drivers and improve the code understanding. On the other hand it requires a significant amount of work in the dma-mapping framework for all architectures, but that's not a big issue for me. Also this doesn't yet solve the vmap() problem that is needed for the kernel virtual mapping. I did try using dma_alloc_writecombine(), but that only works for chunks of 2 MB or smaller, unless I use init_consistent_dma_size() during board setup, which isn't provided for in a DT setup. I couldn't find a better alternative, but I admit I'm not very familiar with all the VM APIs. Do you have any suggestions on how to solve this? Otherwise I'll try and dig in some more. Yes, I'm aware of this issue I'm currently working on solving it. I hope to use standard vmalloc range for all coherent/writecombine allocations and get rid of the custom 'consistent_dma' region at all. Best regards -- Marek Szyprowski Samsung Poland RD Center ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On Wed, Apr 11, 2012 at 12:12:14PM -0600, Stephen Warren wrote: On 04/11/2012 06:10 AM, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. I'll let Jon Mayo comment on the actual driver implementation, since he's a lot more familiar with Tegra's display hardware. However, I have some general comments below. .../devicetree/bindings/gpu/drm/tegra.txt | 24 + arch/arm/mach-tegra/board-dt-tegra20.c |3 + arch/arm/mach-tegra/tegra2_clocks.c|8 +- drivers/gpu/drm/Kconfig|2 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/tegra/Kconfig | 10 + drivers/gpu/drm/tegra/Makefile |5 + drivers/gpu/drm/tegra/tegra_drv.c | 2241 drivers/gpu/drm/tegra/tegra_drv.h | 184 ++ include/drm/tegra_drm.h| 44 + Splitting this patch into two, between arch/arm and drivers/gpu would be a good idea. diff --git a/Documentation/devicetree/bindings/gpu/drm/tegra.txt b/Documentation/devicetree/bindings/gpu/drm/tegra.txt + drm@5420 { + compatible = nvidia,tegra20-drm; This doesn't seem right; there isn't a DRM hardware module on Tegra, since DRM is a Linux/software-specific term. I'd at least expect to see this compatible flag be renamed to something more like nvidia,tegra20-dc (dc==display controller). Since Tegra has two display controller modules (I believe identical?), and numerous other independent(?) blocks, I'd expect to see multiple nodes in device tree, one per hardware block, such that each block gets its own device and driver. That said, I'm not familiar enough with Tegra's display and graphics HW to know if this makes sense. Jon, what's your take here? The clock change below, and in particular the original code there that we use downstream, lends weight to my argument. + reg = 0x5420 0x0004/* display A */ + 0x5424 0x0004/* display B */ + 0x5800 0x0200 ; /* GART aperture */ + interrupts = 0 73 0x04/* display A */ + 0 74 0x04 ; /* display B */ + + lvds { + type = rgb; These sub-nodes probably want a compatible property rather than a type property. + size = 345 194; + + default-mode { + pixel-clock = 61715000; + vertical-refresh = 50; + resolution = 1366 768; + bits-per-pixel = 16; + horizontal-timings = 4 136 2 36; + vertical-timings = 2 4 21 10; + }; + }; I imagine that quite a bit of thought needs to be put into the output part of the binding in order to: * Model the outputs/connectors separately from display controllers. * Make sure that the basic infra-structure for representing an output is general enough to be extensible to all the kinds of outputs we support, not just the LVDS output. * We were wondering about putting an EDID into the DT to represent the display modes, so that all outputs had EDIDs rather than real monitors having EDIDs, and fixed internal displays having some other representation of capabilities. You might want to have a look at the sdrm patches I recently posted to dri-devel and arm Linux Kernel. Among other things they allow to register crtcs/connectors/encoders seperately so that each of them can have its own representation in the devicetree. I haven't looked into devicetree support for DRM, but with or without devicetree the problem that we do not have a single PCI card for registering all DRM components is the same. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On Thursday 12 April 2012, Marek Szyprowski wrote: Scatter lists were initially designed for the disk based block io operations, hence the presence of the in-page offsets and lengths for each chunk. For multimedia use cases providing an array of struct pages and asking dma-mapping to map them into contiguous memory is probably all we need. I wonder if introducing such new calls is a good idea. Anrd, what do think? It will definitely simplify the drivers and improve the code understanding. On the other hand it requires a significant amount of work in the dma-mapping framework for all architectures, but that's not a big issue for me. My feeling is that it's too much like the existing _sg version, so I wouldn't add yet another variant. While having a simple page array is definitely simpler and potentially faster, I think the API is already too complex and we need to be very careful with new additions. Arnd ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Alex Deucher wrote: On Thu, Apr 12, 2012 at 5:33 AM, Thierry Reding thierry.red...@avionic-design.de wrote: In other words I would like to use the Tegra hardware to render content into a framebuffer (using potentially the 3D engine or HW accelerated video decoding blocks) but display that framebuffer with a CRTC registered by a different driver (perhaps provided by a PCIe or USB device). I think such a setup would be possible if the CRTC registration can be decoupled from the DRM driver. Perhaps sdrm even supports that already? You should be able to do something like that already with dma_buf and the drm prime infrastructure. There's even a drm driver for the udl USB framebuffer devices. Using DRM PRIME requires user-space to be involved. I was thinking more along the lines of allowing a dumb DRM driver that only provides a CRTC to register with another driver so that it shows up as an output for the latter DRM device. Then again, having user-space control this may be more flexible. Performance- wise both should be about the same, right? What I don't quite understand yet is how the DMABUF would be synchronized on both ends. Is there some infra- structure to account for it or would I have to export two buffers and flip them during the vblank of the consumer? Thierry pgpa7LMXBkPwG.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [RFC 4/4] drm: Add NVIDIA Tegra support
Hi Thierry, On Thursday, April 12, 2012 3:42 PM Thierry Reding wrote: Also this doesn't yet solve the vmap() problem that is needed for the kernel virtual mapping. I did try using dma_alloc_writecombine(), but that only works for chunks of 2 MB or smaller, unless I use init_consistent_dma_size() during board setup, which isn't provided for in a DT setup. I couldn't find a better alternative, but I admit I'm not very familiar with all the VM APIs. Do you have any suggestions on how to solve this? Otherwise I'll try and dig in some more. Yes, I'm aware of this issue I'm currently working on solving it. I hope to use standard vmalloc range for all coherent/writecombine allocations and get rid of the custom 'consistent_dma' region at all. Does your work aim at removing the 2 MB limitation or at remapping non- contiguous memory to a virtually linear buffer? I have some trouble understanding how removing the 2 MB limit would help, because if all buffers can be allocated contiguously then there wouldn't be any need for the IOMMU, right? I would like to remove 2MiB limitation by moving these mapping to generic vmalloc area. Please notice that this 2MiB limit is only applied to CPU virtual address space and no physical memory is preallocated for consistent dma. ARM linear dma mapping implementation allocates coherent buffers by calling alloc_pages() and then creating a coherent mapping for the allocated buffer. With IOMMU you can allocate chunks of memory and create a contiguous DMA mapping. To fulfill dma mapping request you also need to create a cpu coherent mapping for them and right now my core uses the same remapping function as linear version. The only limitation here will be vmalloc space and its fragmentation. Best regards -- Marek Szyprowski Samsung Poland RD Center ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/12/2012 12:50 AM, Thierry Reding wrote: * Stephen Warren wrote: On 04/11/2012 06:10 AM, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. ... diff --git a/Documentation/devicetree/bindings/gpu/drm/tegra.txt b/Documentation/devicetree/bindings/gpu/drm/tegra.txt ... This doesn't seem right, and couples back to my assertion above that the two display controller modules probably deserve separate device objects, named e.g. tegradc.*. I think I understand where you're going with this. Does the following look more correct? disp1 : dc@5420 { compatible = nvidia,tegra20-dc; reg = 0x5420, 0x0004; interrupts = 0 73 0x04; }; disp2 : dc@5424 { compatible = nvidia,tegra20-dc; reg = 0x5424, 0x0004; interrupts = 0 74 0x04; }; Those look good. drm { compatible = nvidia,tegra20-drm; I'm don't think having an explicit drm node is the right approach; drm is after all a SW term and the DT should be describing HW. Having some kind of top-level node almost certainly makes sense, but naming it something related to tegra display than drm would be appropriate. lvds { compatible = ...; dc = disp1; }; Aren't the outputs separate HW blocks too, such that they would have their own compatible/reg properties and their own drivers, and be outside the top-level drm/display node? I believe the mapping between the output this node represents and the display controller (dc above) that it uses is not static; the connectivity should be set up at runtime, and possibly dynamically alterable via xrandr or equivalent. hdmi { compatible = ...; dc = disp2; }; }; +static int tegra_drm_parse_dt(struct platform_device *pdev) +{ ... + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; ... + dev-platform_data = pdata; I don't think you should assign to dev-platform_data. If you do, then I think the following could happen: * During first probe, the assignment above happens * Module is removed, hence device removed, hence dev-platform_data freed, but not zero'd out * Module is re-inserted, finds that dev-platform_data!=NULL and proceeds to use it. Actually the code does zero out platform_data in tegra_drm_remove(). In fact I did test module unloading and reloading and it works properly. But it should probably be zeroed in case drm_platform_init() fails as well. Instead, the active platform data should probably be stored in a tegra_drm struct that's stored in the dev's private data. tegra_drm_probe() might then look more like: struct tegra_drm *tdev; tdev = devm_kzalloc(); tdev-pdata = pdev-dev.platform_data; if (!tdev-pdata) tdev-pdata = tegra_drm_parse_dt(); if (!tdev-pdata) return -EINVAL; dev_set_drvdata(dev, tdev); This is safe, since probe() will never assume that dev_get_drvdata() might contain something valid before probe() sets it. I prefer my approach over storing the data in an extra field because the device platform_data field is where everybody would expect it. Furthermore this wouldn't be relevant if we decided not to support non-DT setups. Drivers are expected to use pre-existing platform data, if provided. This might happen in order to work around bugs in device tree content. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Alex Deucher wrote: On Thu, Apr 12, 2012 at 10:09 AM, Alex Deucher alexdeuc...@gmail.com wrote: On Thu, Apr 12, 2012 at 9:25 AM, Thierry Reding Then again, having user-space control this may be more flexible. Performance- wise both should be about the same, right? What I don't quite understand yet is how the DMABUF would be synchronized on both ends. Is there some infra- structure to account for it or would I have to export two buffers and flip them during the vblank of the consumer? Performance should be about the same. The tricky part is synchronization. dma_buf is for sharing. There are ideas to make generic sync objects, but for now, it's up to the userspace components to handle it. Some example code: http://lists.freedesktop.org/archives/dri-devel/2012-April/021182.html I saw that post too and it looks really nice. From what you're saying there really doesn't seem to be any advantage to making the connection in the DRM framework. Using DRM PRIME seems like a promising option and it seems to have quite a lot of momentum. Of course I'll need to get the DRM driver up and running properly first. Thierry pgpdZeeKw7Klw.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Stephen Warren wrote: On 04/12/2012 12:50 AM, Thierry Reding wrote: drm { compatible = nvidia,tegra20-drm; I'm don't think having an explicit drm node is the right approach; drm is after all a SW term and the DT should be describing HW. Having some kind of top-level node almost certainly makes sense, but naming it something related to tegra display than drm would be appropriate. In this case there really isn't a HW device that can be represented. But in the end it's still the DRM driver that needs to bind to the device. However the other graphics devices (MPE, VI/CSI, EPP, GR2D and GR3D) probably need to be bound against. Would it be possible for someone at NVIDIA to provide some more details about what those other devices are? GR2D and GR3D seem obvious, MPE might be video decoding, VI/CSI video input and camera interface? As to EPP I have no idea. Maybe one solution would be to have a top-level DRM device with a register map from 0x5400 to 0x547f, which the TRM designates as host registers. Then subnodes could be used for the subdevices. lvds { compatible = ...; dc = disp1; }; Aren't the outputs separate HW blocks too, such that they would have their own compatible/reg properties and their own drivers, and be outside the top-level drm/display node? The RGB output is programmed via the display controller registers. For HDMI, TVO and DSI there are indeed separate sets of registers in addition to the display controller's. So perhaps for those more nodes would be required: hdmi : hdmi@5428 { compatible = nvidia,tegra20-hdmi; reg = 0x5428 0x0004; }; And hook that up with the HDMI output node of the DRM node: drm { hdmi { compatible = ...; connector = hdmi; dc = disp2; }; }; Maybe with this setup we no longer need the compatible property since it will already be inherent in the connector property. There will have to be special handling for the RGB output, which could be the default if the connector property is missing. I believe the mapping between the output this node represents and the display controller (dc above) that it uses is not static; the connectivity should be set up at runtime, and possibly dynamically alterable via xrandr or equivalent. I think the mapping is always static for a given board. There is no way to switch an HDMI port to LVDS at runtime. But maybe I misunderstand what you're saying. Instead, the active platform data should probably be stored in a tegra_drm struct that's stored in the dev's private data. tegra_drm_probe() might then look more like: struct tegra_drm *tdev; tdev = devm_kzalloc(); tdev-pdata = pdev-dev.platform_data; if (!tdev-pdata) tdev-pdata = tegra_drm_parse_dt(); if (!tdev-pdata) return -EINVAL; dev_set_drvdata(dev, tdev); This is safe, since probe() will never assume that dev_get_drvdata() might contain something valid before probe() sets it. I prefer my approach over storing the data in an extra field because the device platform_data field is where everybody would expect it. Furthermore this wouldn't be relevant if we decided not to support non-DT setups. Drivers are expected to use pre-existing platform data, if provided. This might happen in order to work around bugs in device tree content. Okay I see. I'll have to store it in a separate field in the private structure then. Thierry pgpVEjAqQZSMX.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/12/2012 11:44 AM, Thierry Reding wrote: * Stephen Warren wrote: On 04/12/2012 12:50 AM, Thierry Reding wrote: drm { compatible = nvidia,tegra20-drm; I'm don't think having an explicit drm node is the right approach; drm is after all a SW term and the DT should be describing HW. Having some kind of top-level node almost certainly makes sense, but naming it something related to tegra display than drm would be appropriate. In this case there really isn't a HW device that can be represented. But in the end it's still the DRM driver that needs to bind to the device. However the other graphics devices (MPE, VI/CSI, EPP, GR2D and GR3D) probably need to be bound against. Well, everything graphics-related appears to be grouped within some container. In the memory map, there's the range 0x5400-547f, and in the Tegra20 TRM, the first diagram in section 29 Display Controller again lumps all the modules together into one box. I'd say it's perfectly legitimate to create a device tree node to represent this aggregate display/graphics-related engine. So, I think that yes there is a real HW device to represent. And given that, I don't think we should name the node after some OS-specific software concept. Device tree is intended to model hardware. Would it be possible for someone at NVIDIA to provide some more details about what those other devices are? GR2D and GR3D seem obvious, MPE might be video decoding, VI/CSI video input and camera interface? As to EPP I have no idea. MPE is something video encode/decode related. VI/CSI are indeed camera related. I'm afraid I don't personally know any more details than that, and even if I did, I'm only allowed to discuss what's in the TRM:-( I don't think those modules should have much impact on the DRM driver design if that's what you're worried about. I /believe/ they all just interact directly with memory rather than the other components, irrespective of what that first diagram in section 29 might imply. Maybe one solution would be to have a top-level DRM device with a register map from 0x5400 to 0x547f, which the TRM designates as host registers. Then subnodes could be used for the subdevices. Ah yes, just what I was thinking above:-) I don't think you'd /have/ to make the nodes sub-nodes; you could use phandles to refer from the top-level node to the other components. One might end up having to use phandles anyway to represent the routing from DCA or DCB to MIPI or HDMI anyway, so it's possible that using phandles everywhere might be simpler and more consistent than parent/child relationships for some things and phandles for other things. lvds { compatible = ...; dc = disp1; }; Aren't the outputs separate HW blocks too, such that they would have their own compatible/reg properties and their own drivers, and be outside the top-level drm/display node? The RGB output is programmed via the display controller registers. For HDMI, TVO and DSI there are indeed separate sets of registers in addition to the display controller's. So perhaps for those more nodes would be required: hdmi : hdmi@5428 { compatible = nvidia,tegra20-hdmi; reg = 0x5428 0x0004; }; Yes, looks reasonable. And hook that up with the HDMI output node of the DRM node: drm { hdmi { compatible = ...; connector = hdmi; dc = disp2; }; }; Maybe with this setup we no longer need the compatible property since it will already be inherent in the connector property. There will have to be special handling for the RGB output, which could be the default if the connector property is missing. I suspect you'd have something more like: tegra-graphics { output-resources = hdmi tvo dsi ... ; display-controllers = disp1 disp2; }; i.e. just a list of all extant devices. Each should provide some common API so you could just map from phandle to of_node to device object, and call the appropriate APIs on it. Finally note that although I didn't really represent it correct above, there are at least 3 levels of object/hierarchy here: Display controllers reads from RAM and form a set of pixels to display. I believe things like cursors, overlays, pan-scan, etc. are resolved entirely at this level. Output resources drive certain pins on Tegra in some specific format such as HDMI, VGA, etc. I think all output resources can actually display the data from either display controller. Probably even 2 ORs can show an image from the same DC at once for mirrored display. Finally, there are the user connectors. I suspect it's plausible for muxes to exist between the ORs and user connectors, although that's probably a lot less likely. I believe this is the level at which to represent things like which I2C bus is used for
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Daniel Vetter wrote: On Wed, Apr 11, 2012 at 03:23:26PM +0200, Thierry Reding wrote: * Daniel Vetter wrote: On Wed, Apr 11, 2012 at 02:10:30PM +0200, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. Indeed, after a quick look there are tons of functions that are just stubs ;-) One thing I wonder though is why you directly use the iommu api and not wrap it up into dma_map? Is arm infrastructure just not there yet or do you plan to tightly integrate the tegra drm with the iommu (e.g. for process space switching or similarly funky stuff)? I'm not sure I know what you are referring to. Looking for all users of iommu_map() doesn't turn up anything related to dma_map. Can you point me in the right direction? Well, you use the iommu api to map/unmap memory into the iommu for tegra, whereas usually device drivers just use the dma api to do that. The usual interface is dma_map_sg/dma_unmap_sg, but there are quite a few variants around. I'm just wondering why this you've choosen this. I don't think this works on ARM. Maybe I'm not seeing the whole picture but judging by a quick look through the kernel tree there aren't any users that map DMA memory through an IOMMU. Maybe your question is answered by my reply to Alan's comment. The mapping is actually done to get a linear view for the display controller which doesn't support SG transfers. The kernel and user-space already have virtual linear buffers. Perhaps I'm being dense? Thierry pgpCSMPp2Bykj.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
Hm, in that case it looks like your iommu works more like the gtt on intel chips Don't overgeneralize there - on the GMA500/600 the GTT doesn't allow CPU side access of the GTT map (ie you can't use it to linearise pages for CPU view) and the 3600 is even stranger Alan ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
Maybe your question is answered by my reply to Alan's comment. The mapping is actually done to get a linear view for the display controller which doesn't support SG transfers. The kernel and user-space already have virtual linear buffers. The framebuffer currently needs a physically contiguous map for the console devices. Well you could vmap them but that is pretty hideous on a 32bit platform with 32bit 1080p display plugged into it! Alan ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Daniel Vetter wrote: On Wed, Apr 11, 2012 at 04:11:08PM +0200, Thierry Reding wrote: * Daniel Vetter wrote: On Wed, Apr 11, 2012 at 03:23:26PM +0200, Thierry Reding wrote: * Daniel Vetter wrote: On Wed, Apr 11, 2012 at 02:10:30PM +0200, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. Indeed, after a quick look there are tons of functions that are just stubs ;-) One thing I wonder though is why you directly use the iommu api and not wrap it up into dma_map? Is arm infrastructure just not there yet or do you plan to tightly integrate the tegra drm with the iommu (e.g. for process space switching or similarly funky stuff)? I'm not sure I know what you are referring to. Looking for all users of iommu_map() doesn't turn up anything related to dma_map. Can you point me in the right direction? Well, you use the iommu api to map/unmap memory into the iommu for tegra, whereas usually device drivers just use the dma api to do that. The usual interface is dma_map_sg/dma_unmap_sg, but there are quite a few variants around. I'm just wondering why this you've choosen this. I don't think this works on ARM. Maybe I'm not seeing the whole picture but judging by a quick look through the kernel tree there aren't any users that map DMA memory through an IOMMU. Maybe your question is answered by my reply to Alan's comment. The mapping is actually done to get a linear view for the display controller which doesn't support SG transfers. The kernel and user-space already have virtual linear buffers. Hm, in that case it looks like your iommu works more like the gtt on intel chips and less like the iommu on intel platforms (which we access through the dma_map api). Yes, it's very much like the GTT on Intel chips. In fact I've been using the gma500 driver as a source for inspiration. Wikipedia confirms that GTT and GART are synonymous. I wonder whether that will end up in some layering fun together with dma_buf, which conceptually is at the same level as the dma api, which usually uses an underlying iommu exposed with the iommu api you're using. That's odd. The only users of the IOMMU API that I can find in the kernel tree are in drivers/remoteproc and drivers/media/video/omap3isp. And omap3isp doesn't do any actual mapping at a quick glance. Can you point me to where this is hooked up with the Intel IOMMU? Thierry pgpzu3IiL9Ype.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
* Alan Cox wrote: Maybe your question is answered by my reply to Alan's comment. The mapping is actually done to get a linear view for the display controller which doesn't support SG transfers. The kernel and user-space already have virtual linear buffers. The framebuffer currently needs a physically contiguous map for the console devices. Well you could vmap them but that is pretty hideous on a 32bit platform with 32bit 1080p display plugged into it! Heh, vmap() is exactly what I do. =) Would you mind explaining why exactly it is hideous? I'll have to investigate what an appropriate alternative would look like. Thierry pgp5cCIucNT6d.pgp Description: PGP signature ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
Heh, vmap() is exactly what I do. =) Would you mind explaining why exactly it is hideous? On x86 we don't have a vast amount of address space available for virtual remappings and the framebuffer then eats over 8MB of it. The ideal case is that the fb layer can be taught to do page/offset addressing nicely. At that point we'd be able to attach the text consoles to arbitary GEM objects.. which means we can do really cool stuff. Alan ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On Wednesday 11 April 2012, Thierry Reding wrote: * Daniel Vetter wrote: On Wed, Apr 11, 2012 at 03:23:26PM +0200, Thierry Reding wrote: * Daniel Vetter wrote: On Wed, Apr 11, 2012 at 02:10:30PM +0200, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. Indeed, after a quick look there are tons of functions that are just stubs ;-) One thing I wonder though is why you directly use the iommu api and not wrap it up into dma_map? Is arm infrastructure just not there yet or do you plan to tightly integrate the tegra drm with the iommu (e.g. for process space switching or similarly funky stuff)? I'm not sure I know what you are referring to. Looking for all users of iommu_map() doesn't turn up anything related to dma_map. Can you point me in the right direction? Well, you use the iommu api to map/unmap memory into the iommu for tegra, whereas usually device drivers just use the dma api to do that. The usual interface is dma_map_sg/dma_unmap_sg, but there are quite a few variants around. I'm just wondering why this you've choosen this. I don't think this works on ARM. Maybe I'm not seeing the whole picture but judging by a quick look through the kernel tree there aren't any users that map DMA memory through an IOMMU. dma_map_sg is certainly the right interface to use, and Marek Szyprowski has patches to make that work on ARM, hopefully going into v3.5, so you could use those. Arnd ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/11/2012 06:10 AM, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. I'll let Jon Mayo comment on the actual driver implementation, since he's a lot more familiar with Tegra's display hardware. However, I have some general comments below. .../devicetree/bindings/gpu/drm/tegra.txt | 24 + arch/arm/mach-tegra/board-dt-tegra20.c |3 + arch/arm/mach-tegra/tegra2_clocks.c|8 +- drivers/gpu/drm/Kconfig|2 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/tegra/Kconfig | 10 + drivers/gpu/drm/tegra/Makefile |5 + drivers/gpu/drm/tegra/tegra_drv.c | 2241 drivers/gpu/drm/tegra/tegra_drv.h | 184 ++ include/drm/tegra_drm.h| 44 + Splitting this patch into two, between arch/arm and drivers/gpu would be a good idea. diff --git a/Documentation/devicetree/bindings/gpu/drm/tegra.txt b/Documentation/devicetree/bindings/gpu/drm/tegra.txt + drm@5420 { + compatible = nvidia,tegra20-drm; This doesn't seem right; there isn't a DRM hardware module on Tegra, since DRM is a Linux/software-specific term. I'd at least expect to see this compatible flag be renamed to something more like nvidia,tegra20-dc (dc==display controller). Since Tegra has two display controller modules (I believe identical?), and numerous other independent(?) blocks, I'd expect to see multiple nodes in device tree, one per hardware block, such that each block gets its own device and driver. That said, I'm not familiar enough with Tegra's display and graphics HW to know if this makes sense. Jon, what's your take here? The clock change below, and in particular the original code there that we use downstream, lends weight to my argument. + reg = 0x5420 0x0004/* display A */ + 0x5424 0x0004/* display B */ + 0x5800 0x0200 ; /* GART aperture */ + interrupts = 0 73 0x04/* display A */ +0 74 0x04 ; /* display B */ + + lvds { + type = rgb; These sub-nodes probably want a compatible property rather than a type property. + size = 345 194; + + default-mode { + pixel-clock = 61715000; + vertical-refresh = 50; + resolution = 1366 768; + bits-per-pixel = 16; + horizontal-timings = 4 136 2 36; + vertical-timings = 2 4 21 10; + }; + }; I imagine that quite a bit of thought needs to be put into the output part of the binding in order to: * Model the outputs/connectors separately from display controllers. * Make sure that the basic infra-structure for representing an output is general enough to be extensible to all the kinds of outputs we support, not just the LVDS output. * We were wondering about putting an EDID into the DT to represent the display modes, so that all outputs had EDIDs rather than real monitors having EDIDs, and fixed internal displays having some other representation of capabilities. I'm hoping that Jon will drive this. diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c - PERIPH_CLK(disp1, tegradc.0,NULL, 27, 0x138, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ - PERIPH_CLK(disp2, tegradc.1,NULL, 26, 0x13c, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ + PERIPH_CLK(disp1, tegra-drm,NULL, 27, 0x138, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ + PERIPH_CLK(disp2, tegra-drm,NULL, 26, 0x13c, 6, mux_pllp_plld_pllc_clkm, MUX), /* scales with voltage and process_id */ This doesn't seem right, and couples back to my assertion above that the two display controller modules probably deserve separate device objects, named e.g. tegradc.*. diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig new file mode 100644 index 000..f3382c9 --- /dev/null +++ b/drivers/gpu/drm/tegra/Kconfig @@ -0,0 +1,10 @@ +config DRM_TEGRA + tristate NVIDIA Tegra + depends on DRM ARCH_TEGRA Jon, do you think we'll end up eventually having a unified