On Fri, May 4, 2018 at 11:16 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > Quoting Daniel Vetter (2018-05-04 09:57:59) >> On Fri, May 04, 2018 at 09:31:33AM +0100, Chris Wilson wrote: >> > Quoting Daniel Vetter (2018-05-04 09:23:01) >> > > On Fri, May 04, 2018 at 10:17:22AM +0200, Daniel Vetter wrote: >> > > > On Fri, May 04, 2018 at 09:09:10AM +0100, Chris Wilson wrote: >> > > > > Quoting Daniel Vetter (2018-05-03 15:25:52) >> > > > > > Almost everyone uses dma_fence_default_wait. >> > > > > > >> > > > > > v2: Also remove the BUG_ON(!ops->wait) (Chris). >> > > > > >> > > > > I just don't get the rationale for implicit over explicit. >> > > > >> > > > Closer approximation of dwim semantics. There's been tons of patch series >> > > > all over drm and related places to get there, once we have a big pile of >> > > > implementations and know what the dwim semantics should be. Individually >> > > > they're all not much, in aggregate they substantially simplify simple >> > > > drivers. >> > > >> > > I also think clearer separation between optional optimization hooks and >> > > mandatory core parts is useful in itself. >> > >> > A new spelling of midlayer ;) I don't see the contradiction with a >> > driver saying use the default and simplicity. (I know which one the >> > compiler thinks is simpler ;) >> >> If the compiler overhead is real then I guess it would makes to be >> explicit. I don't expect that to be a problem though for a blocking >> function. >> >> I disagree on this being a midlayer - you can still overwrite everything >> you please to. What it does help is people doing less copypasting (and >> assorted bugs), at least in the grand scheme of things. And we do have a >> _lot_ more random small drivers than just a few years ago. Reducing the >> amount of explicit typing just to get default bahaviour has been an >> ongoing theme for a few years now, and your objection here is about the >> first that this is not a good idea. So I'm somewhat confused. > > I'm just saying I don't see any rationale for this patch. > > "Almost everyone uses dma_fence_default_wait." > > Why change? > > Making it look simpler on the surface, so that you don't have to think > about things straight away? I understand the appeal, but I do worry > about it just being an illusion. (Cutting and pasting a line saying > .wait = default_wait, doesn't feel that onerous, as you likely cut and > paste the ops anyway, and at the very least you are reminded about some > of the interactions. You could even have default initializers and/or > magic macros to hide the cut and paste; maybe a simple_dma_fence [now > that's a midlayer!] but I haven't looked.) In really monolithic vtables like drm_driver we do use default function macros, so you type 1 line, get them all. But dma_fence_ops is pretty small, and most drivers only implement a few callbacks. Also note that e.g. the ->release callback already works like that, so this pattern is there already. I simply extended it to ->wait and ->enable_signaling. Also note that I leave the EXPORT_SYMBOL in place, you can still wrap dma_fence_default_wait if you wish to do so. But I just realized that I didn't clean out the optional release hooks, I guess I should do that too (for the few cases it's not yet done) and respin. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
