On Tue, Oct 15, 2019 at 09:13:37PM +0300, Ville Syrjälä wrote: > On Tue, Oct 15, 2019 at 07:48:29PM +0200, Daniel Vetter wrote: > > On Tue, Oct 15, 2019 at 7:28 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote: > > > > > > Hi Daniel > > > > > > Am 15.10.19 um 16:33 schrieb Daniel Vetter: > > > > Hi Thomas, > > > > > > > > On Mon, Oct 14, 2019 at 04:04:01PM +0200, Thomas Zimmermann wrote: > > > >> (was: DRM driver for fbdev devices) > > > >> > > > >> This is version 2 of the fbdev conversion helpers. It's more or less a > > > >> rewrite of the original patchset. > > > >> > > > >> The fbdev subsystem is considered legacy and will probably be removed at > > > >> some point. This would mean the loss of a signifanct number of drivers. > > > >> Some of the affected hardware is not in use any longer, but some hardware > > > >> is still around and provides good(-enough) framebuffers. > > > >> > > > >> The fbconv helpers allow for running the current DRM stack on top of fbdev > > > >> drivers. It's a set of functions that convert between fbdev interfaces and > > > >> DRM interfaces. Based on SHMEM and simple KMS helpers, it only offers the > > > >> basic functionality of a framebuffer, but should be compatible with most > > > >> existing fbdev drivers. > > > >> > > > >> A DRM driver using fbconv helpers consists of > > > >> > > > >> * DRM stub code that calls into fbconv helpers, and > > > >> * the original fbdev driver code. > > > >> > > > >> The fbdev driver code has to be modified to register itself with the > > > >> stub driver instead of the fbdev core framework. A tutorial on how to use > > > >> the helpers is part of this patchset. The resulting driver hybrid can be > > > >> refactored into a first-class DRM driver. The fbconv helpers contain a > > > >> number of comments, labeled 'DRM porting note', which explain the required > > > >> steps. > > > >> > > > >> I tested the current patchset with the following drivers: atyfb, aty128fb, > > > >> matroxfb, pm2fb, pm3fb, rivafb, s3fb, savagefb, sisfb, tdfxfb and tridentfb. > > > >> With each, I was able to successfully start with fbcon enabled, run weston and > > > >> X11. The drivers are available at [1]. For reference, the patchset includes > > > >> the Matrox stub driver. > > > > > > > > So I really don't want to rain on the parade here, since if you think this > > > > is useful when converting fbdev drivers I'll buy that, and I'm all for > > > > getting more modern drivers into drm. > > > > > > > > But I have a bunch of concerns with the approach you're proposing here: > > > > > > > > - we've tried staging for drm driver refactoring, it hurts. Separate tree > > > > plus the quick pace in refactoring create lots of pains. And for small > > > > drivers refacotoring before it's not buying you anything above > > > > refactoring in your own personal tree. And for big drivers we're fairly > > > > lenient with merging drivers that aren't fully polished yet, if there's > > > > a team serious enough with cleaning up the mess. I think even merging > > > > partial drivers directly under drivers/gpu (but behind CONFIG_BROKEN) is > > > > better than staging. > > > > > > I mostly put this into staging, because it's the kind of code you'd > > > expect there. > > > > Yeah, except we tried, it's a real pain. Conclusion by everyone > > involved is that staging doesn't work for the drm subsystem. > > > > > > - we've had conversion helpers before (for the legacy kms -> atomic > > > > upgrade). They constantly broke, pretty much every release when someone > > > > wanted to use them they first had to fix them up again. I think having > > > > those helpers is good, but better to just have them in some branch > > > > somewhere where it's clear that they might not work anymore on latest > > > > upstream. > > > > > > > > - especially for some of these simple fbdev drivers I feel like just > > > > typing a new driver from scratch might be simpler. > > > > > > > > A few more concerns specifically for your mga example: > > > > > > > > - We already have a mga driver. Might be better to integrate support for > > > > older mgas into that than have a parallel driver. > > > > > > Two colleagues of mine, Takashi and Egbert, send a patch that added > > > support for desktop G200s to mgag200. [1] But it was rejected because > > > the devices are two old and not relevant any longer. If that opinion has > > > changed in the meantime, I wouldn't mind adding support for desktop GPUs > > > to the driver. > > > > Hm that seems to have petered out inconclusive. I definitely think a > > merged mga driver is better than 2 drm atomic kms drivers for roughly > > the same hardware. I'm also assuming that at least for now no one > > plans to resurrect the 3d acceleration support for these old chips. > > But even then it's fairly easy to disable all that on the server > > chips. > > > > > > - Your helper is based on simple display pipe, and I think for these old > > > > mga chips (especially the dual pipe mga 450 and 550) simple display pipe > > > > helper is more a hindering detour than actual help. From a quick read > > > > through the code (especially all the custom ioctls) you definitely want > > > > separate TV-out connector to expose all the tv mode properties (instead > > > > of the custom ioctls). > > > > > > Around the G100, there's something like a change in generation. Before, > > > devices had only a single output and less than 8 MiB of RAM. This works > > > well with GEM SHMEM and simple KMS. Afterwards, devices have 8 MiB or > > > more and multiple outputs. GEM VRAM and the full set of helpers fit this > > > much better. Maybe having 2 drivers that share common code (or 3 with > > > the Server Engine chipsets) makes most sense. > > > > Yeah if that's the case maybe a mga100 and mga200g driver fits better. > > Former based on simple display pipe. > > The display hardware differences are quite minimal from > 1064SG to G200. G400 did add the second CRTC but essentially > nothing else changed from G200 display. G450/G550 changed > the PLLS around a bit just for the heck of it, and integrated > the TMDS transmitter and TV encoder. And then they did even > more PLL madness with the different G200 server chip variants. > > So IMO from display hw POV G100 vs. G200 split doesn't really > make sense. Ah, I did forget that G100 and earlier don't support the cursor 16 color mode that G200+ have. So I guess there is a little bit of a difference there. -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
