On Sun, Feb 14, 2016 at 12:22 PM, Jerome Glisse <j.glisse@xxxxxxxxx> wrote: >> There's still a bunch of legacy stuff in these patches that's on our list of things to refactor. Some of that is >> - dc/adapter >> - dc/asic_capability >> - dc/audio >> - dc/bios >> - dc/gpio >> >> We should be able to cut the size of this code to about 1/3 of what it is now. >> >> As for the LOC we have about >> 22k for HW programming >> 30k legacy stuff >> 6k dc/calcs - autogenerated from formulas provided by HW team >> 15k includes >> 6k amdgpu_dm >> 8k dc/core >> >> About 14k of those are blank lines (we have a habit of leaving lots of blank space) and 16k are comments. > > Cleaning up that is not enough, abstracting kernel API like kmalloc or i2c, > or similar, is a no go. If the current drm infrastructure does not suit your > need then you need to work on improving it to suit your need. You can not > redevelop a whole drm layer inside your code and expect to upstream it. > > Linux device driver are about sharing infrastructure and trying to expose > it through common API to userspace. > > So i strongly suggest that you start thinking on how to change the drm API > to suit your need and to start discussions about those changes. If you need > them then they will likely be usefull to others down the road. A bit more context on why OS abstractions layers like dal here are massive frowned upon: First reason is that it makes live painful for everyone who tries to refactor across the entire subsystem (which I do a lot, so that's what I care about). For that you need to be able to understand every driver fairly quickly, and that's only possible if all drivers use the same concepts. Which means no abstraction layer, but directly using&extending core drm constructs like drm_framebuffer/crtc/encoder/bridge/connector/display_information. Even if your dal concepts match 1:1 with those, if the link is only at runtime or through pointers instead of through subclassing everything becomes much more painful to understand. Note that this isn't a reason against hw/driver specific additional concepts (all big drivers have those), but against undue amounts of insolation between drm concepts and driver concepts. The other bit is the so-called midlayer design mistake, https://lwn.net/Articles/336262/ which states that wedging an entire such abstraction layer will also inevitably lead to bad design overall (neglecting the practical troubles is causes for everyone else). DRM suffered badly from the midlayer of old days due to compat with *bsd in the core, which took years to fix up (and is still not completely done). And we only ever merged 2 drivers who had midlayers like dal, and both had special reasons: - exynos was one of the very first armsoc kms drivers, and merged pretty much for goodwill. Google was eventually fed up with the midlayer in upstream and paid Collabora to nuke it. - omapdrm reused the display support code already merged into upstream in the fbdev subsytem, and that's why it had a midlayer. But Tomi is now also in the process of removing this. So even if all the cleanup is done (removal of duplicated code and the OS abstraction layers) there's some really big reservations against DAL as a concept itself. That was essentially what's Dave "why do we need this" was all about. And to answer that you need in my opinion better reasons "shared abstraction with $otheros/thing/whatever", since if that's the only reason you argue against 20+ years of shared experience in linux device driver development that this is a very bad one. Note again that this isn't against introducing additional amd-speicific display concepts in the amd driver, or existing concepts. Every driver does that. This is just against putting an entire abstraction layer in-between drm core and your real display driver. The other big issue is that upstream really puts a lot of weight onto continuity - no forking or wholesale rewrites every 1-2 years, instead gradual change of what's already there. And I suspect the amount of effort to get dal into a reasonable shape is on par with just adding the new features you want to amdgpu directly one-by-one. Starting out with amdgpu also has the added benefit that there's lots of howtos and experience with converting an existing kms driver to atomic. We don't really have that for DAL->atomic, and from reading through the code I couldn't convince myself that the current code is really atomic. Largely that's due to not using drm core concepts and structures, but remapping things. Anyway, I figured I type in a few more words to explain some of the reasons behind the resistance to massive abstraction layers in kernel device drivers. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
