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. Thierry
Attachment:
signature.asc
Description: PGP signature
