Hi Christian, First off, thanks for reviving this effort. It's been one of the things that I've had nagging at me for much too long and I think it needs to be solved. So I'm hopeful that the more people we get looking at this the more likely it will be to come up with a solution that works well for everyone.
That said, I don't agree with the approach you've chosen here. I'll try to clarify why below. On Sun, Oct 11, 2015 at 05:09:21PM +0200, Christian Gmeiner wrote: > This commit adds a generic renderonly driver library, which fullfille > the requirements for tegra and etnaviv. As a result it is possible to > run unmodified egl software directly (without any compositor) on > supported devices. Technically this isn't a library but rather a midlayer. There's a subtle difference, but the implications are what concerns me. Back when I wrote the original driver for Tegra/Nouveau I also looked into possibilities to make this more generic. Since I know how bad mid- layers can be (from kernel experience) I shied away from something like this early on. What I tried to do next was abstract away enough to make this usable by more than just a single driver. Unfortunately the end result was that not much could be reused, so drivers ended up still having to implement all of the pipe_* objects, only to call generic functions. Most of the code needed in the various callbacks ended up not being much more than just a single line, so the gains from a helper library weren't very big. Another reason why I think this level of abstraction doesn't gain us much is that we already have a good level of abstraction, which is Gallium. I realize that implementing only the skeleton for a full Gallium driver is rather complicated, but that's due to the fact that graphics drivers are complex beasts. That said, I think for some areas it might be beneficial to have helpers to reduce the amount of duplication. However I think at this point in time we haven't had enough real-world exposure for this kind of driver to know what the requirements are. For that reason I think it is premature to use a generic midlayer such as this. Yes, I know that the alternative is roughly 2000 lines of code per driver, but on one hand that's nothing compared to the amount of code required by a proper GPU driver and on the other hand this will (ideally) be temporary until we get a better picture of where things need to go. At which point it may become more obvious how we can solve the boilerplate problem while at the same time avoiding the restrictions imposed by the midlayer. > In every use case we import a dumb buffer from scanout gpu into > the renderonly gpu. > > If the scanout hardware does support the used tiling format from the > renderonly gpu, a driver can define a function which is used to 'setup' > the needed tiling on that imported buffer. This functions gets called > during rendertarget resource creation. > > If the scanout hardware does not support the used tiling format we need > to create an extra rendertarget resource for the renderonly gpu. > During XXX we blit the renderonly rendertarget onto the imported dumb > buffer. > > We assume that the renderonly driver provides a blit function that is > capable of resolving the tilied into untiled one. I understand that there's a want to eliminate the amount of boilerplate, but I think this approach of using a midlayer has several flaws. One of the typical pitfalls with a midlayer such as this is that it has the potential to grow into an unmaintainable mess. Granted, this currently doesn't look all that bad, but that's primarily because it supports only two types of devices. I suspect that the more devices we add, the more hooks and quirks we'll need. Every combination of GPU and display is likely going to have their own specialties that need to be handled and which are beyond simple things like the tiling format. We also know that there are issues with the current approach (EGL clients in Weston don't properly display). It's unknown what the reason for this is and it may require largish changes to the architecture to fix it. For all of the above reasons I think it'd be better to live with a little boilerplate for now and refactor things as they become obvious candidates for refactoring. [...] > diff --git a/src/gallium/drivers/renderonly/renderonly_screen.c > b/src/gallium/drivers/renderonly/renderonly_screen.c [...] > +static const char * > +renderonly_get_vendor(struct pipe_screen *pscreen) > +{ > + return "renderonly"; > +} I don't think this going to do us much good. Applications may want to know more precisely what kind of device they're running on and change behaviour accordingly. > +static void renderonly_screen_destroy(struct pipe_screen *pscreen) > +{ > + struct renderonly_screen *screen = to_renderonly_screen(pscreen); > + > + screen->gpu->destroy(screen->gpu); > + free(pscreen); > +} > + > +static int > +renderonly_screen_get_param(struct pipe_screen *pscreen, enum pipe_cap param) > +{ > + struct renderonly_screen *screen = to_renderonly_screen(pscreen); > + > + return screen->gpu->get_param(screen->gpu, param); > +} > > +static float > +renderonly_screen_get_paramf(struct pipe_screen *pscreen, enum pipe_capf > param) > +{ > + struct renderonly_screen *screen = to_renderonly_screen(pscreen); > + > + return screen->gpu->get_paramf(screen->gpu, param); > +} I can easily imagine cases where the drivers might want to override some of these parameters rather than simply propagate the result from the GPU driver. > +static boolean > +renderonly_screen_is_format_supported(struct pipe_screen *pscreen, > + enum pipe_format format, > + enum pipe_texture_target target, > + unsigned sample_count, > + unsigned usage) > +{ > + struct renderonly_screen *screen = to_renderonly_screen(pscreen); > + > + return screen->gpu->is_format_supported(screen->gpu, format, target, > + sample_count, usage); > +} Same here. I'm almost certain that GPUs will support more formats than the display hardware (in general). Also I guess this depends a lot on the usage parameter as well. I know that this is a copy of what I had in my original proposal, but I fully admit that it wasn't much more than a proof of concept with quite a few rough edges to work out. Thierry
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev