On 31 January 2017 at 21:17, Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > On Tue, Jan 31, 2017 at 10:49:51AM -0500, Sean Paul wrote: >> On Tue, Jan 31, 2017 at 04:02:26PM +0100, Thierry Reding wrote: >> > On Tue, Jan 31, 2017 at 09:38:53AM -0500, Sean Paul wrote: >> > > On Tue, Jan 31, 2017 at 09:54:49AM +0100, Thierry Reding wrote: >> > > > On Tue, Jan 31, 2017 at 09:01:07AM +0900, Inki Dae wrote: >> > > > > >> > > > > >> > > > > 2017년 01월 24일 10:50에 Hoegeun Kwon 이(가) 쓴 글: >> > > > > > Dear Thierry, >> > > > > > >> > > > > > Could you please review this patch? >> > > > > >> > > > > Thierry, I think this patch has been reviewed enough but no comment >> > > > > from you. Seems you are busy. I will pick up this. >> > > > >> > > > Sorry, but that's not how it works. This patch has gone through 8 >> > > > revisions within 4 weeks, and I tend to ignore patches like that until >> > > > the dust settles. >> > > > >> > > >> > > Seems like the dust was pretty settled. It was posted on 1/11, pinged on 1/24, >> > > and picked up on 1/31. I don't think it's unreasonable to take it through >> > > another tree after that. >> > > >> > > I wonder if drm_panel would benefit from the -misc group maintainership model >> > > as drm_bridge does. By spreading out the workload, the high-maintenance >> > > patches would hopefully find someone to shepherd them through. >> > >> > Except that nobody except me really cares. >> >> I certainly haven't been paying attention. My excuse, at least, is that you're a >> great maintainer and I haven't thought the patches need a second look. Perhaps >> if we moved towards a group, more people would be vested/care? > > I doubt that group maintainership would change much about the lack of > peer review. Peer review makes maintenance a lot easier. Usually when I > see that a patch has been reviewed by someone that I think I can trust, > I only give it a very brief look before applying. However, if there's > been no other review I need to take a lot more time to review. > >> > If we let people take patches >> > through separate trees or group-maintained trees they'll likely go in >> > without too much thought. DRM panel is somewhat different from core DRM >> > in this regard because its infrastructure is minimal and there's little >> > outside the panel-simple driver. So we're still at a stage where we need >> > to fine-tune what drivers should look like and how we can improve. >> > >> >> Fair point. With drm_bridge, I've been lending review help, but deferring to >> Archit for merge on patches which I think he should look at. Gustavo is in a >> similar place with fences. drm_panel seems like something that should follow >> the same model. Maybe once more people (or one person) get up to speed on >> things, we could share the load. > > I certainly wouldn't mind more people reviewing panel patches. Applying > them is the easy part. > >> > > > Other than that, this continues the same madness that I've repeatedly >> > > > complained about in the past. The whole mechanism of running through a >> > > > series of writes and not caring about errors until the very end is >> > > > something we've discussed at length in the past. It also in large parts >> > > > duplicates a bunch of functions from other Samsung panel drivers that I >> > > > already said should eventually be moved to something saner. >> > > > >> > > >> > > FWIW, this type of error handling isn't my preference either. If we must defer, >> > > I'd rather not keep it in ctx, but rather pass around an argument so it's more >> > > obvious we need to deal with it in the return. That said, this seems like >> > > a case of letting the perfect be the enemy of the good, surely something is >> > > better than nothing? >> > >> > That's what I ended up saying the last two times. But this has got to >> > stop at some point. If you look at most of the panel drivers, they look >> > more like material for the staging tree rather than DRM proper. >> > >> > Yes, something is better than nothing, but we can't have this multiply >> > further. >> > >> >> So perhaps if we had more reviewers, we could tighten up the review feedback >> loop and avoid this while still getting things merged? > > Maybe. Like I said, I would very much welcome more review on panel > patches. > I think that there's a part that Inki and/or others might have forgotten. Being the sole/core person reviewing this leads to a bit too often repeat of the same principles ... which after a while gets a bit [pick your favourite non-cool word]. In one of my experiences [whist attempting to help out], one had to repeat/reword the exact same thing three times in order to be addressed :-\ -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
