From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board Date: Tue, 31 Jan 2017 10:49:51 -0500 Message-ID: <20170131154951.GZ20076@art_vandelay> References: <1484116439-7275-1-git-send-email-hoegeun.kwon@samsung.com> <1484116439-7275-3-git-send-email-hoegeun.kwon@samsung.com> <08c5d94b-c76f-af14-c08f-478e26a34a7c@samsung.com> <588FD3C3.7080508@samsung.com> <20170131085449.GA19348@ulmo.ba.sec> <20170131143853.GU20076@art_vandelay> <20170131150226.GB4519@ulmo.ba.sec> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20170131150226.GB4519@ulmo.ba.sec> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org, Donghwa Lee , linux-kernel@vger.kernel.org, andi.shyti@samsung.com, jh80.chung@samsung.com, cw00.choi@samsung.com, kgene@kernel.org, dri-devel@lists.freedesktop.org, Hyungwon Hwang , Krzysztof Kozlowski , Hoegeun Kwon List-Id: linux-samsung-soc@vger.kernel.org T24gVHVlLCBKYW4gMzEsIDIwMTcgYXQgMDQ6MDI6MjZQTSArMDEwMCwgVGhpZXJyeSBSZWRpbmcg d3JvdGU6Cj4gT24gVHVlLCBKYW4gMzEsIDIwMTcgYXQgMDk6Mzg6NTNBTSAtMDUwMCwgU2VhbiBQ YXVsIHdyb3RlOgo+ID4gT24gVHVlLCBKYW4gMzEsIDIwMTcgYXQgMDk6NTQ6NDlBTSArMDEwMCwg VGhpZXJyeSBSZWRpbmcgd3JvdGU6Cj4gPiA+IE9uIFR1ZSwgSmFuIDMxLCAyMDE3IGF0IDA5OjAx OjA3QU0gKzA5MDAsIElua2kgRGFlIHdyb3RlOgo+ID4gPiA+IAo+ID4gPiA+IAo+ID4gPiA+IDIw MTfrhYQgMDHsm5QgMjTsnbwgMTA6NTDsl5AgSG9lZ2V1biBLd29uIOydtCjqsIApIOyTtCDquIA6 Cj4gPiA+ID4gPiBEZWFyIFRoaWVycnksCj4gPiA+ID4gPiAKPiA+ID4gPiA+IENvdWxkIHlvdSBw bGVhc2UgcmV2aWV3IHRoaXMgcGF0Y2g/Cj4gPiA+ID4gCj4gPiA+ID4gVGhpZXJyeSwgSSB0aGlu ayB0aGlzIHBhdGNoIGhhcyBiZWVuIHJldmlld2VkIGVub3VnaCBidXQgbm8gY29tbWVudAo+ID4g PiA+IGZyb20geW91LiBTZWVtcyB5b3UgYXJlIGJ1c3kuIEkgd2lsbCBwaWNrIHVwIHRoaXMuCj4g PiA+IAo+ID4gPiBTb3JyeSwgYnV0IHRoYXQncyBub3QgaG93IGl0IHdvcmtzLiBUaGlzIHBhdGNo IGhhcyBnb25lIHRocm91Z2ggOAo+ID4gPiByZXZpc2lvbnMgd2l0aGluIDQgd2Vla3MsIGFuZCBJ IHRlbmQgdG8gaWdub3JlIHBhdGNoZXMgbGlrZSB0aGF0IHVudGlsCj4gPiA+IHRoZSBkdXN0IHNl dHRsZXMuCj4gPiA+IAo+ID4gCj4gPiBTZWVtcyBsaWtlIHRoZSBkdXN0IHdhcyBwcmV0dHkgc2V0 dGxlZC4gSXQgd2FzIHBvc3RlZCBvbiAxLzExLCBwaW5nZWQgb24gMS8yNCwKPiA+IGFuZCBwaWNr ZWQgdXAgb24gMS8zMS4gSSBkb24ndCB0aGluayBpdCdzIHVucmVhc29uYWJsZSB0byB0YWtlIGl0 IHRocm91Z2gKPiA+IGFub3RoZXIgdHJlZSBhZnRlciB0aGF0Lgo+ID4gCj4gPiBJIHdvbmRlciBp ZiBkcm1fcGFuZWwgd291bGQgYmVuZWZpdCBmcm9tIHRoZSAtbWlzYyBncm91cCBtYWludGFpbmVy c2hpcCBtb2RlbAo+ID4gYXMgZHJtX2JyaWRnZSBkb2VzLiBCeSBzcHJlYWRpbmcgb3V0IHRoZSB3 b3JrbG9hZCwgdGhlIGhpZ2gtbWFpbnRlbmFuY2UKPiA+IHBhdGNoZXMgd291bGQgaG9wZWZ1bGx5 IGZpbmQgc29tZW9uZSB0byBzaGVwaGVyZCB0aGVtIHRocm91Z2guCj4gCj4gRXhjZXB0IHRoYXQg bm9ib2R5IGV4Y2VwdCBtZSByZWFsbHkgY2FyZXMuIAoKSSBjZXJ0YWlubHkgaGF2ZW4ndCBiZWVu IHBheWluZyBhdHRlbnRpb24uIE15IGV4Y3VzZSwgYXQgbGVhc3QsIGlzIHRoYXQgeW91J3JlIGEK Z3JlYXQgbWFpbnRhaW5lciBhbmQgSSBoYXZlbid0IHRob3VnaHQgdGhlIHBhdGNoZXMgbmVlZCBh IHNlY29uZCBsb29rLiBQZXJoYXBzCmlmIHdlIG1vdmVkIHRvd2FyZHMgYSBncm91cCwgbW9yZSBw ZW9wbGUgd291bGQgYmUgdmVzdGVkL2NhcmU/Cgo+IElmIHdlIGxldCBwZW9wbGUgdGFrZSBwYXRj aGVzCj4gdGhyb3VnaCBzZXBhcmF0ZSB0cmVlcyBvciBncm91cC1tYWludGFpbmVkIHRyZWVzIHRo ZXknbGwgbGlrZWx5IGdvIGluCj4gd2l0aG91dCB0b28gbXVjaCB0aG91Z2h0LiBEUk0gcGFuZWwg aXMgc29tZXdoYXQgZGlmZmVyZW50IGZyb20gY29yZSBEUk0KPiBpbiB0aGlzIHJlZ2FyZCBiZWNh dXNlIGl0cyBpbmZyYXN0cnVjdHVyZSBpcyBtaW5pbWFsIGFuZCB0aGVyZSdzIGxpdHRsZQo+IG91 dHNpZGUgdGhlIHBhbmVsLXNpbXBsZSBkcml2ZXIuIFNvIHdlJ3JlIHN0aWxsIGF0IGEgc3RhZ2Ug d2hlcmUgd2UgbmVlZAo+IHRvIGZpbmUtdHVuZSB3aGF0IGRyaXZlcnMgc2hvdWxkIGxvb2sgbGlr ZSBhbmQgaG93IHdlIGNhbiBpbXByb3ZlLgo+IAoKRmFpciBwb2ludC4gV2l0aCBkcm1fYnJpZGdl LCBJJ3ZlIGJlZW4gbGVuZGluZyByZXZpZXcgaGVscCwgYnV0IGRlZmVycmluZyB0bwpBcmNoaXQg Zm9yIG1lcmdlIG9uIHBhdGNoZXMgd2hpY2ggSSB0aGluayBoZSBzaG91bGQgbG9vayBhdC4gR3Vz dGF2byBpcyBpbiBhCnNpbWlsYXIgcGxhY2Ugd2l0aCBmZW5jZXMuIGRybV9wYW5lbCBzZWVtcyBs aWtlIHNvbWV0aGluZyB0aGF0IHNob3VsZCBmb2xsb3cKdGhlIHNhbWUgbW9kZWwuIE1heWJlIG9u Y2UgbW9yZSBwZW9wbGUgKG9yIG9uZSBwZXJzb24pIGdldCB1cCB0byBzcGVlZCBvbgp0aGluZ3Ms IHdlIGNvdWxkIHNoYXJlIHRoZSBsb2FkLgoKPiA+ID4gT3RoZXIgdGhhbiB0aGF0LCB0aGlzIGNv bnRpbnVlcyB0aGUgc2FtZSBtYWRuZXNzIHRoYXQgSSd2ZSByZXBlYXRlZGx5Cj4gPiA+IGNvbXBs YWluZWQgYWJvdXQgaW4gdGhlIHBhc3QuIFRoZSB3aG9sZSBtZWNoYW5pc20gb2YgcnVubmluZyB0 aHJvdWdoIGEKPiA+ID4gc2VyaWVzIG9mIHdyaXRlcyBhbmQgbm90IGNhcmluZyBhYm91dCBlcnJv cnMgdW50aWwgdGhlIHZlcnkgZW5kIGlzCj4gPiA+IHNvbWV0aGluZyB3ZSd2ZSBkaXNjdXNzZWQg YXQgbGVuZ3RoIGluIHRoZSBwYXN0LiBJdCBhbHNvIGluIGxhcmdlIHBhcnRzCj4gPiA+IGR1cGxp Y2F0ZXMgYSBidW5jaCBvZiBmdW5jdGlvbnMgZnJvbSBvdGhlciBTYW1zdW5nIHBhbmVsIGRyaXZl cnMgdGhhdCBJCj4gPiA+IGFscmVhZHkgc2FpZCBzaG91bGQgZXZlbnR1YWxseSBiZSBtb3ZlZCB0 byBzb21ldGhpbmcgc2FuZXIuCj4gPiA+IAo+ID4gCj4gPiBGV0lXLCB0aGlzIHR5cGUgb2YgZXJy b3IgaGFuZGxpbmcgaXNuJ3QgbXkgcHJlZmVyZW5jZSBlaXRoZXIuIElmIHdlIG11c3QgZGVmZXIs Cj4gPiBJJ2QgcmF0aGVyIG5vdCBrZWVwIGl0IGluIGN0eCwgYnV0IHJhdGhlciBwYXNzIGFyb3Vu ZCBhbiBhcmd1bWVudCBzbyBpdCdzIG1vcmUKPiA+IG9idmlvdXMgd2UgbmVlZCB0byBkZWFsIHdp dGggaXQgaW4gdGhlIHJldHVybi4gVGhhdCBzYWlkLCB0aGlzIHNlZW1zIGxpa2UKPiA+IGEgY2Fz ZSBvZiBsZXR0aW5nIHRoZSBwZXJmZWN0IGJlIHRoZSBlbmVteSBvZiB0aGUgZ29vZCwgc3VyZWx5 IHNvbWV0aGluZyBpcwo+ID4gYmV0dGVyIHRoYW4gbm90aGluZz8KPiAKPiBUaGF0J3Mgd2hhdCBJ IGVuZGVkIHVwIHNheWluZyB0aGUgbGFzdCB0d28gdGltZXMuIEJ1dCB0aGlzIGhhcyBnb3QgdG8K PiBzdG9wIGF0IHNvbWUgcG9pbnQuIElmIHlvdSBsb29rIGF0IG1vc3Qgb2YgdGhlIHBhbmVsIGRy aXZlcnMsIHRoZXkgbG9vawo+IG1vcmUgbGlrZSBtYXRlcmlhbCBmb3IgdGhlIHN0YWdpbmcgdHJl ZSByYXRoZXIgdGhhbiBEUk0gcHJvcGVyLgo+IAo+IFllcywgc29tZXRoaW5nIGlzIGJldHRlciB0 aGFuIG5vdGhpbmcsIGJ1dCB3ZSBjYW4ndCBoYXZlIHRoaXMgbXVsdGlwbHkKPiBmdXJ0aGVyLgo+ CgpTbyBwZXJoYXBzIGlmIHdlIGhhZCBtb3JlIHJldmlld2Vycywgd2UgY291bGQgdGlnaHRlbiB1 cCB0aGUgcmV2aWV3IGZlZWRiYWNrCmxvb3AgYW5kIGF2b2lkIHRoaXMgd2hpbGUgc3RpbGwgZ2V0 dGluZyB0aGluZ3MgbWVyZ2VkPwoKSSdtIG1vc3RseSB0aGlua2luZyBvdXQgbG9hZCBoZXJlLCBz byB0YWtlIGl0IGZvciB3aGF0IGl0J3Mgd29ydGguCgpTZWFuCgoKCj4gTGFzdCB0aW1lIHdlIGRp c2N1c3NlZCB0aGlzIHRoZXJlIHdhcyBzb21lIHJvdWdoIGNvbmNlbnN1cyB0aGF0IHRoZQo+IGlu aXRpYWxpemF0aW9uIHNlcXVlbmNlIHdvdWxkIGJlIHNwbGl0IGludG8gc2VwYXJhdGUgZnVuY3Rp b25zLCB3aGljaCBpcwo+IGFscmVhZHkgbW9zdGx5IHRydWUgZm9yIHRoZXNlIGRyaXZlcnMsIGFu ZCB0aGVuIHNldHRsZSBvbiBhIGNvbW1vbgo+IHNpZ25hdHVyZSBmb3IgdGhlc2UgZnVuY3Rpb25z LCB3aGljaCBpcyBhbHNvIG1vc3RseSB0aGUgY2FzZSBhbHJlYWR5LAo+IGFuZCB0aGVuIGhhdmUg YSB0YWJsZSBvZiBmdW5jdGlvbnMgdGhhdCBjYW4gYmUgY2FsbGVkIG9uZSBhZnRlciBhbm90aGVy Cj4gd2l0aCBlcnJvciBoYW5kbGluZyBvbiBlYWNoIGNhbGwuIFRoYXQgd2F5IGF0IGxlYXN0IHlv dSBjYW4gcHJvdmlkZQo+IGRpYWdub3N0aWNzIGFib3V0IHdoYXQgZnVuY3Rpb24gZmFpbGVkLCBh bmQgeW91IGNhbiBhYm9ydCBlYXJseSBiZWNhdXNlCj4gd2UgaGF2ZSB0byBhc3N1bWUgdGhhdCBm YWlsdXJlcyBhcmUgZmF0YWwuCj4gCj4gVGhpZXJyeQoKCgo+IF9fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fCj4gZHJpLWRldmVsIG1haWxpbmcgbGlzdAo+IGRy aS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKPiBodHRwczovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAoKCi0tIApTZWFuIFBhdWwsIFNvZnR3YXJl IEVuZ2luZWVyLCBHb29nbGUgLyBDaHJvbWl1bSBPUwpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1h bi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751336AbdAaPuP (ORCPT ); Tue, 31 Jan 2017 10:50:15 -0500 Received: from mail-qt0-f175.google.com ([209.85.216.175]:34015 "EHLO mail-qt0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751059AbdAaPtz (ORCPT ); Tue, 31 Jan 2017 10:49:55 -0500 Date: Tue, 31 Jan 2017 10:49:51 -0500 From: Sean Paul To: Thierry Reding Cc: devicetree@vger.kernel.org, linux-samsung-soc@vger.kernel.org, kgene@kernel.org, Donghwa Lee , linux-kernel@vger.kernel.org, andi.shyti@samsung.com, cw00.choi@samsung.com, jh80.chung@samsung.com, dri-devel@lists.freedesktop.org, Hyungwon Hwang , Krzysztof Kozlowski , Hoegeun Kwon Subject: Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board Message-ID: <20170131154951.GZ20076@art_vandelay> References: <1484116439-7275-1-git-send-email-hoegeun.kwon@samsung.com> <1484116439-7275-3-git-send-email-hoegeun.kwon@samsung.com> <08c5d94b-c76f-af14-c08f-478e26a34a7c@samsung.com> <588FD3C3.7080508@samsung.com> <20170131085449.GA19348@ulmo.ba.sec> <20170131143853.GU20076@art_vandelay> <20170131150226.GB4519@ulmo.ba.sec> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170131150226.GB4519@ulmo.ba.sec> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? > 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. > > > 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? I'm mostly thinking out load here, so take it for what it's worth. Sean > Last time we discussed this there was some rough concensus that the > initialization sequence would be split into separate functions, which is > already mostly true for these drivers, and then settle on a common > signature for these functions, which is also mostly the case already, > and then have a table of functions that can be called one after another > with error handling on each call. That way at least you can provide > diagnostics about what function failed, and you can abort early because > we have to assume that failures are fatal. > > Thierry > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS