Re: [Mesa-dev] [PATCH v3 5/6] tegra: Initial support
On Thu, Mar 01, 2018 at 09:10:39AM -0800, Dylan Baker wrote: > Quoting Thierry Reding (2018-03-01 05:54:53) > > --- /dev/null > > +++ b/src/gallium/winsys/tegra/drm/meson.build > > @@ -0,0 +1,33 @@ > > +# Copyright © 2018 NVIDIA CORPORATION > > + > > +# Permission is hereby granted, free of charge, to any person obtaining a > > copy > > +# of this software and associated documentation files (the "Software"), to > > deal > > +# in the Software without restriction, including without limitation the > > rights > > +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > +# copies of the Software, and to permit persons to whom the Software is > > +# furnished to do so, subject to the following conditions: > > + > > +# The above copyright notice and this permission notice shall be included > > in > > +# all copies or substantial portions of the Software. > > + > > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS > > OR > > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > THE > > +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > > FROM, > > +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > > IN THE > > +# SOFTWARE. > > + > > +libtegradrm = static_library( > > + 'tegradrm', > > + 'tegra_drm_winsys.c', > > + include_directories : [ > > +inc_include, inc_src, inc_gallium, inc_gallium_aux, > > inc_gallium_drivers, > > +inc_gallium_winsys > > + ], > > +) > > + > > +driver_tegra = declare_dependency( > > + compile_args : '-DGALLIUM_TEGRA', > > + link_with : libtegradrm, > > +) > > We don't need this driver_tegra, since the one in drivers/tegra is actually > complete, right? > > Dylan Yeah, I think you're right. I don't remember why I added this. Let me run some build tests without it. Thanks, Thierry signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 5/6] tegra: Initial support
On Thu, Mar 01, 2018 at 09:41:37AM -0500, Ilia Mirkin wrote: > Is this substantially different than the renderonly helper? i.e. could > you reuse it somehow? (If not, that's fine too, just asking.) Yeah, this is fairly different from renderonly in that it completely wraps another driver. renderonly was derived from an earlier version of this patch and simplified this by actually returning the GPU screen from the scanout "frontend" driver. This requires changing the GPU driver to know about a potential scanout engine because the GPU driver needs to call into the scanout driver when creating resources in order for the scanout driver to allocate or import the resources so that it is able to scan them out. This reversal makes things very simple, but at the same time limits the flexibility because the "frontend" driver becomes essentially a slave to the GPU driver. The other disadvantage is that the GPU driver needs to be aware of the scanout part to make it all work. Note that there are cases where this is absolutely fine. If all you care about is passing buffers between scanout and render engines, renderonly is good enough. Or if that's all you can do, then renderonly will do a good job. The wrapper driver has the same goal of making split scanout/render setups behave as any other combined scanout/render setup, so that applications don't have to be made aware of the split and encapsulate the mechanism of how buffers are shared between GPU and scanout within Mesa. But, in my opinion, there are some advantages to it: firstly, the situation on Tegra is somewhat different from other SoCs in that the same GPU driver (Nouveau) is also used on combined scanout/render devices. So modifying it to talk to an external scanout engine seems a little tacked on. Secondly, having a wrapper around Nouveau allows us to implement policy specific to Tegra. For example, some formats or modifiers may work well on a desktop GPU but perform very badly on Tegra. The wrapper driver gives us more flexibility in filtering out such combinations and exposing Tegra-specific capabilities if need be. Thirdly we have a couple of other hardware units on Tegra that we may want to use in a Mesa driver (such as VIC for image composition). These units are not available to Nouveau, but via the host1x interface that is accessed using the Tegra DRM device node. The wrapper driver makes it easy to intercept some operations and redirect them to VIC if we know that they will be better suited to its capabilities. Technically we could reuse the renderonly helper, but we'd be limiting ourselves to using Nouveau underneath. We'd also need to modify Nouveau to become aware of non-GPU aspects of Tegra and we'd be unable to use non-GPU hardware units to offload some operations. Those are all fairly big restrictions, so the wrapper driver is the preferred solution. > Also, will this work with the following code in nvc0_resource_fence > (which I recently discovered does *not* play nice with the trace > driver): > >struct nvc0_screen *screen = nvc0_screen(res->base.screen); > > i.e. will the resource's screen be the nvc0 screen or the tegra > screen? [for resources passed into the nvc0 driver obviously] Yes, this should work fine. I've occasionally seen crashes caused by the wrapper handing the wrong resources to nvc0, but I'm fairly confident that these are all fixed now. I was able to run a fairly wide range of applications, on all of bare-metal, X and Wayland, including glxgears, glmark2, kmscube, Weston (with some of the sample applications), some NVIDIA internal demos as well as a source port of Doom 3[0]. The way this is achieved is that the Tegra driver wraps every resources upon allocation and storing a pointer to the nvc0 resource created by Nouveau. When these resources are used, the Tegra driver unwraps the resources and passes the pointer to the nvc0 resource to the nvc0 context. Thierry [0]: https://github.com/dhewm/dhewm3 signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 5/6] tegra: Initial support
Quoting Thierry Reding (2018-03-01 05:54:53) > Tegra K1 and later use a GPU that can be driven by the Nouveau driver. > But the GPU is a pure render node and has no display engine, hence the > scanout needs to happen on the Tegra display hardware. The GPU and the > display engine each have a separate DRM device node exposed by the > kernel. > > To make the setup appear as a single device, this driver instantiates > a Nouveau screen with each instance of a Tegra screen and forwards GPU > requests to the Nouveau screen. For purposes of scanout it will import > buffers created on the GPU into the display driver. Handles that > userspace requests are those of the display driver so that they can be > used to create framebuffers. > > This has been tested with some GBM test programs, as well as kmscube and > weston. All of those run without modifications, but I'm sure there is a > lot that can be improved. > > Some fixes contributed by Hector Martin . > Also please make sure that any new meson.build files are in the EXTRA_DIST array in the companion Makefile.am so that they'll be in the dist tarball. Dylan signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 5/6] tegra: Initial support
Quoting Thierry Reding (2018-03-01 05:54:53) > --- /dev/null > +++ b/src/gallium/winsys/tegra/drm/meson.build > @@ -0,0 +1,33 @@ > +# Copyright © 2018 NVIDIA CORPORATION > + > +# Permission is hereby granted, free of charge, to any person obtaining a > copy > +# of this software and associated documentation files (the "Software"), to > deal > +# in the Software without restriction, including without limitation the > rights > +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > +# copies of the Software, and to permit persons to whom the Software is > +# furnished to do so, subject to the following conditions: > + > +# The above copyright notice and this permission notice shall be included in > +# all copies or substantial portions of the Software. > + > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE > +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > FROM, > +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > THE > +# SOFTWARE. > + > +libtegradrm = static_library( > + 'tegradrm', > + 'tegra_drm_winsys.c', > + include_directories : [ > +inc_include, inc_src, inc_gallium, inc_gallium_aux, inc_gallium_drivers, > +inc_gallium_winsys > + ], > +) > + > +driver_tegra = declare_dependency( > + compile_args : '-DGALLIUM_TEGRA', > + link_with : libtegradrm, > +) We don't need this driver_tegra, since the one in drivers/tegra is actually complete, right? Dylan signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
