From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Norris Subject: Re: [PATCH v6 3/3] drm/rockchip: Add ROCKCHIP DW MIPI DSI controller driver Date: Wed, 6 Dec 2017 13:52:43 -0800 Message-ID: <20171206215243.GB14257@google.com> References: <1512551301-12946-1-git-send-email-nickey.yang@rock-chips.com> <1512551301-12946-4-git-send-email-nickey.yang@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1512551301-12946-4-git-send-email-nickey.yang@rock-chips.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Nickey Yang Cc: mark.rutland@arm.com, airlied@linux.ie, hoegsberg@gmail.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, philippe.cornu@st.com, yannick.fertre@st.com, linux-rockchip@lists.infradead.org, robh+dt@kernel.org, laurent.pinchart@ideasonboard.com, zyw@rock-chips.com, xbl@rock-chips.com, mka@chromium.org, hl@rock-chips.com List-Id: linux-rockchip.vger.kernel.org SGkgTmlja2V5LCBvdGhlcnMsCgpJIGp1c3Qgd2FudCB0byBoaWdobGlnaHQgYSB0aGluZyBvciB0 d28gaGVyZS4gT3RoZXJ3aXNlLCBteQonUmV2aWV3ZWQtYnknIHN0aWxsIGJhc2ljYWxseSBzdGFu ZHMgKEZXSVcpLgoKT24gV2VkLCBEZWMgMDYsIDIwMTcgYXQgMDU6MDg6MjFQTSArMDgwMCwgTmlj a2V5IFlhbmcgd3JvdGU6Cj4gQWRkIHRoZSBST0NLQ0hJUCBEU0kgY29udHJvbGxlciBkcml2ZXIg dGhhdCB1c2VzIHRoZSBTeW5vcHN5cyBEZXNpZ25XYXJlCj4gTUlQSSBEU0kgaG9zdCBjb250cm9s bGVyIGJyaWRnZS4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBOaWNrZXkgWWFuZyA8bmlja2V5LnlhbmdA cm9jay1jaGlwcy5jb20+Cj4gU2lnbmVkLW9mZi1ieTogQnJpYW4gTm9ycmlzIDxicmlhbm5vcnJp c0BjaHJvbWl1bS5vcmc+Cj4gUmV2aWV3ZWQtYnk6IEJyaWFuIE5vcnJpcyA8YnJpYW5ub3JyaXNA Y2hyb21pdW0ub3JnPgo+IFJldmlld2VkLWJ5OiBTZWFuIFBhdWwgPHNlYW5wYXVsQGNocm9taXVt Lm9yZz4KPiAtLS0KPiBjaGFuZ2U6Cj4gCj4gdjI6Cj4gICAgYWRkIGVycl9wbGxyZWYsIHJlbW92 ZSB1bm5lY2Vzc2FyeSBlbmNvZGVyLmVuYWJsZSAmIGRpc2FibGUKPiAgICBjb3JyZWN0IHNwZWxs aW5nIG1pc3Rha2VzCj4gdjM6Cj4gICAgY2FsbCBkd19taXBpX2RzaV91bmJpbmQoKSBpbiBkd19t aXBpX2RzaV9yb2NrY2hpcF91bmJpbmQoKQo+ICAgIGZpeCB0eXBvLCB1c2Ugb2ZfZGV2aWNlX2dl dF9tYXRjaF9kYXRhKCksCj4gICAgY2hhbmdlIHNvbWUg4oCYYmluZCgp4oCZIGxvZ2ljIGludG8g J3Byb2JlKCknCj4gICAgYWRkICdkZXZfc2V0X2RydmRhdGEoKScKPiB2NDoKPiAgIHJldHVybiAt RUlOVkFMIHdoZW4gY2FuIG5vdCBnZXQgYmVzdF9mcmVxCj4gICBhZGQgYSBjbGFyaWZ5aW5nIGNv bW1lbnQgd2hlbiBnZXQgdmNvCj4gICBhZGQgcmV2aWV3IHRhZwo+IHY1Ogo+ICAga2VlcCBvdXIg cG93ZXIgZG9tYWluIGVuYWJsZWQgd2hpbGUgdG91Y2hpbmcgR1JGCj4gdjY6Cj4gICBjaGFuZ2Ug ZnVuYyBkd19taXBpX2VuY29kZXJfZGlzYWJsZSBuYW1lIHRvCj4gICBkd19taXBpX2RzaV9lbmNv ZGVyX2Rpc2FibGUKCldlIG5vdGljZWQgYSByZWdyZXNzaW9uIHcuci50LiBwbV9ydW50aW1lXyoo KSBoYW5kbGluZyB1c2luZyB0aGlzIHBhdGNoLApoZW5jZSB0aGUgcG1fcnVudGltZSBjaGFuZ2Vz IGluIHY1L3Y2LiBXZSBhY3R1YWxseSBuZWVkIHRvIGtlZXAgb3VyCnBvd2VyIGRvbWFpbiBlbmFi bGVkIGluIHRoZSBtb2RlX3NldCgpIGZ1bmN0aW9uLCB3aGVyZSB3ZSBzdGFydCB0bwpjb25maWd1 cmUgc29tZSBSb2NrY2hpcC1zcGVjaWZpYyByZWdpc3RlcnMgKEdSRikuIE1vcmUgb24gdGhhdCBi ZWxvdy4KCj4gCj4gIGRyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9LY29uZmlnICAgICAgICAgICAg ICAgIHwgICAgMiArLQo+ICBkcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvTWFrZWZpbGUgICAgICAg ICAgICAgICB8ICAgIDIgKy0KPiAgZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3LW1pcGktZHNp LmMgICAgICAgICAgfCAxMzQ5IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCj4gIGRyaXZlcnMvZ3B1 L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaV9yb2NrY2hpcC5jIHwgIDc4NSArKysrKysrKysrKysr Cj4gIGRyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fZHJ2LmMgICAgIHwgICAg MiArLQo+ICBkcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJtX2Rydi5oICAgICB8 ICAgIDIgKy0KPiAgNiBmaWxlcyBjaGFuZ2VkLCA3ODkgaW5zZXJ0aW9ucygrKSwgMTM1MyBkZWxl dGlvbnMoLSkKPiAgZGVsZXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9k dy1taXBpLWRzaS5jCj4gIGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL2dwdS9kcm0vcm9ja2No aXAvZHctbWlwaS1kc2lfcm9ja2NoaXAuYwo+IAoKLi4uCgo+IGRpZmYgLS1naXQgYS9kcml2ZXJz L2dwdS9kcm0vcm9ja2NoaXAvZHctbWlwaS1kc2lfcm9ja2NoaXAuYyBiL2RyaXZlcnMvZ3B1L2Ry bS9yb2NrY2hpcC9kdy1taXBpLWRzaV9yb2NrY2hpcC5jCj4gbmV3IGZpbGUgbW9kZSAxMDA2NDQK PiBpbmRleCAwMDAwMDAwLi42NmFiNmZlCj4gLS0tIC9kZXYvbnVsbAo+ICsrKyBiL2RyaXZlcnMv Z3B1L2RybS9yb2NrY2hpcC9kdy1taXBpLWRzaV9yb2NrY2hpcC5jCj4gQEAgLTAsMCArMSw3ODUg QEAKCi4uLgoKPiArc3RhdGljIHZvaWQgZHdfbWlwaV9kc2lfZW5jb2Rlcl9tb2RlX3NldChzdHJ1 Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIsCj4gKwkJCQkJIHN0cnVjdCBkcm1fZGlzcGxheV9tb2Rl ICptb2RlLAo+ICsJCQkJCSBzdHJ1Y3QgZHJtX2Rpc3BsYXlfbW9kZSAqYWRqdXN0ZWQpCj4gK3sK PiArCXN0cnVjdCBkd19taXBpX2RzaV9yb2NrY2hpcCAqZHNpID0gdG9fZHNpKGVuY29kZXIpOwo+ ICsJY29uc3Qgc3RydWN0IHJvY2tjaGlwX2R3X2RzaV9jaGlwX2RhdGEgKmNkYXRhID0gZHNpLT5j ZGF0YTsKPiArCWludCB2YWwsIHJldCwgbXV4Owo+ICsKPiArCW11eCA9IGRybV9vZl9lbmNvZGVy X2FjdGl2ZV9lbmRwb2ludF9pZChkc2ktPmRldi0+b2Zfbm9kZSwKPiArCQkJCQkJJmRzaS0+ZW5j b2Rlcik7Cj4gKwlpZiAobXV4IDwgMCkKPiArCQlyZXR1cm47Cj4gKwkvKgo+ICsJICogRm9yIHRo ZSBSSzMzOTksIHRoZSBjbGsgb2YgZ3JmIG11c3QgYmUgZW5hYmxlZCBiZWZvcmUgd3JpdGluZyBn cmYKPiArCSAqIHJlZ2lzdGVyLiBBbmQgZm9yIFJLMzI4OCBvciBvdGhlciBzb2MsIHRoaXMgZ3Jm X2NsayBtdXN0IGJlIE5VTEwsCj4gKwkgKiB0aGUgY2xrX3ByZXBhcmVfZW5hYmxlIHJldHVybiB0 cnVlIGRpcmVjdGx5Lgo+ICsJICovCj4gKwlyZXQgPSBjbGtfcHJlcGFyZV9lbmFibGUoZHNpLT5n cmZfY2xrKTsKPiArCWlmIChyZXQpIHsKPiArCQlEUk1fREVWX0VSUk9SKGRzaS0+ZGV2LCAiRmFp bGVkIHRvIGVuYWJsZSBncmZfY2xrOiAlZFxuIiwgcmV0KTsKPiArCQlyZXR1cm47Cj4gKwl9Cj4g KwlwbV9ydW50aW1lX2dldF9zeW5jKGRzaS0+ZGV2KTsKCldoYXQgaGFwcGVucyBpZiB0aGVyZSdz IGEgY2xrX3ByZXBhcmVfZW5hYmxlKCkgZmFpbHVyZSBvciBmYWlsdXJlIHRvCnJldHJpZXZlIHRo ZSBlbmRwb2ludCBJRCBlYXJsaWVyIGluIHRoaXMgZnVuY3Rpb24/IFlvdSB3b24ndCBjYWxsCnBt X3J1bnRpbWVfZ2V0XyooKS4uLmJ1dCBtaWdodCB3ZSBzdGlsbCBzZWUgYSBjYWxsIHRvCmR3X21p cGlfZHNpX2VuY29kZXJfZGlzYWJsZSgpLCB3aGljaCB3b3VsZCBtZWFuIHdlIGdldCB1bmJhbGFu Y2VkCnJ1bnRpbWUgUE0gc3RhdHVzPwoKQWxzbyAoYW5kIG1vcmUgaW1wb3J0YW50bHkpLCBpcyBp dCBmYWlyIHRvIGRvIGFsbCBvZiB0aGlzIGluIG1vZGVfc2V0KCk/CkkgYmVsaWV2ZSBBcmNoaXQg YXNrZWQgYWJvdXQgdGhpcyBiZWZvcmUsIGFuZCB0aGUgcmVhc29uIHdlJ3JlIGRvaW5nCnRoaXMg c3R1ZmYgaW4gbW9kZV9zZXQoKSBub3cgKHdoZXJlIHByZXZpb3VzbHksIHRoZSBSb2NrY2hpcCBk cml2ZXIgd2FzCmRvaW5nIGl0IGluIC0+ZW5hYmxlKCkpIGlzIGJlY2F1c2Ugd2hlbiBQaGlsaXBw ZSBleHRyYWN0ZWQgdGhlIHN5bm9wc3lzCmJyaWRnZSBkcml2ZXIsIHRoYXQgY29kZSBtaWdyYXRl ZCB0byAtPm1vZGVfc2V0KCkuCgpCdXQsIEknbSByZWFkaW5nIHRoZSBjb21tZW50cyBvbiBkcm1f ZW5jb2Rlcl9oZWxwZXJfZnVuY3M6Om1vZGVfc2V0LCBhbmQKSSBzZWU6CgogICAgICAgIC8qKgog ICAgICAgICAqIEBtb2RlX3NldDoKICAgICAgICAgKgogICAgICAgICAqIFRoaXMgY2FsbGJhY2sg aXMgdXNlZCB0byB1cGRhdGUgdGhlIGRpc3BsYXkgbW9kZSBvZiBhbiBlbmNvZGVyLgogICAgICAg ICAqCiAgICAgICAgICogTm90ZSB0aGF0IHRoZSBkaXNwbGF5IHBpcGUgaXMgY29tcGxldGVseSBv ZmYgd2hlbiB0aGlzIGZ1bmN0aW9uIGlzCiAgICAgICAgICogY2FsbGVkLiBEcml2ZXJzIHdoaWNo IG5lZWQgaGFyZHdhcmUgdG8gYmUgcnVubmluZyBiZWZvcmUgdGhleSBwcm9ncmFtCiAgICAgICAg ICogdGhlIG5ldyBkaXNwbGF5IG1vZGUgKGJlY2F1c2UgdGhleSBpbXBsZW1lbnQgcnVudGltZSBQ TSkgc2hvdWxkIG5vdAogICAgICAgICAqIHVzZSB0aGlzIGhvb2ssIGJlY2F1c2UgdGhlIGhlbHBl ciBsaWJyYXJ5IGNhbGxzIGl0IG9ubHkgb25jZSBhbmQgbm90CiAgICAgICAgICogZXZlcnkgdGlt ZSB0aGUgZGlzcGxheSBwaXBlbGluZSBpcyBzdXNwZW5kIHVzaW5nIGVpdGhlciBEUE1TIG9yIHRo ZQogICAgICAgICAqIG5ldyAiQUNUSVZFIiBwcm9wZXJ0eS4gU3VjaCBkcml2ZXJzIHNob3VsZCBp bnN0ZWFkIG1vdmUgYWxsIHRoZWlyCiAgICAgICAgICogZW5jb2RlciBzZXR1cCBpbnRvIHRoZSBA ZW5hYmxlIGNhbGxiYWNrLgoKVGhhdCBzb3VuZHMgbGlrZSBwZXJoYXBzIHRoZSBzeW5vcHN5cyBk cml2ZXIgc2hvdWxkIGJlIG1vdmluZyB0byB1c2UKLT5lbmFibGUoKSBpbnN0ZWFkLCBzbyB0aGUg Um9ja2NoaXAgZHJpdmVyIGNhbiBkbyB0aGF0IGFzIHdlbGw/CgpBdCBhbnkgcmF0ZSwgSSBoYXZl bid0IGZvdW5kIGFueSByZWFsIHByb2JsZW0gd2l0aCB1c2luZyBtb2RlX3NldCgpIHNvCmZhciwg YnV0IEkgd2FzIGp1c3QgY3VyaW91cyB3aGF0IHRoZSByZWNvbW1lbmRlZCBwcmFjdGljZSB3YXMu Cgo+ICsJdmFsID0gY2RhdGEtPmRzaTBfZW5fYml0IDw8IDE2Owo+ICsJaWYgKG11eCkKPiArCQl2 YWwgfD0gY2RhdGEtPmRzaTBfZW5fYml0Owo+ICsJcmVnbWFwX3dyaXRlKGRzaS0+Z3JmX3JlZ21h cCwgY2RhdGEtPmdyZl9zd2l0Y2hfcmVnLCB2YWwpOwo+ICsKPiArCWlmIChjZGF0YS0+Z3JmX2Rz aTBfbW9kZV9yZWcpCj4gKwkJcmVnbWFwX3dyaXRlKGRzaS0+Z3JmX3JlZ21hcCwgY2RhdGEtPmdy Zl9kc2kwX21vZGVfcmVnLAo+ICsJCQkgICAgIGNkYXRhLT5ncmZfZHNpMF9tb2RlKTsKPiArCj4g KwljbGtfZGlzYWJsZV91bnByZXBhcmUoZHNpLT5ncmZfY2xrKTsKPiArfQo+ICsKPiArc3RhdGlj IGludAo+ICtkd19taXBpX2RzaV9lbmNvZGVyX2F0b21pY19jaGVjayhzdHJ1Y3QgZHJtX2VuY29k ZXIgKmVuY29kZXIsCj4gKwkJCQkgc3RydWN0IGRybV9jcnRjX3N0YXRlICpjcnRjX3N0YXRlLAo+ ICsJCQkJIHN0cnVjdCBkcm1fY29ubmVjdG9yX3N0YXRlICpjb25uX3N0YXRlKQo+ICt7Cj4gKwlz dHJ1Y3Qgcm9ja2NoaXBfY3J0Y19zdGF0ZSAqcyA9IHRvX3JvY2tjaGlwX2NydGNfc3RhdGUoY3J0 Y19zdGF0ZSk7Cj4gKwlzdHJ1Y3QgZHdfbWlwaV9kc2lfcm9ja2NoaXAgKmRzaSA9IHRvX2RzaShl bmNvZGVyKTsKPiArCj4gKwlzd2l0Y2ggKGRzaS0+Zm9ybWF0KSB7Cj4gKwljYXNlIE1JUElfRFNJ X0ZNVF9SR0I4ODg6Cj4gKwkJcy0+b3V0cHV0X21vZGUgPSBST0NLQ0hJUF9PVVRfTU9ERV9QODg4 Owo+ICsJCWJyZWFrOwo+ICsJY2FzZSBNSVBJX0RTSV9GTVRfUkdCNjY2Ogo+ICsJCXMtPm91dHB1 dF9tb2RlID0gUk9DS0NISVBfT1VUX01PREVfUDY2NjsKPiArCQlicmVhazsKPiArCWNhc2UgTUlQ SV9EU0lfRk1UX1JHQjU2NToKPiArCQlzLT5vdXRwdXRfbW9kZSA9IFJPQ0tDSElQX09VVF9NT0RF X1A1NjU7Cj4gKwkJYnJlYWs7Cj4gKwlkZWZhdWx0Ogo+ICsJCVdBUk5fT04oMSk7Cj4gKwkJcmV0 dXJuIC1FSU5WQUw7Cj4gKwl9Cj4gKwo+ICsJcy0+b3V0cHV0X3R5cGUgPSBEUk1fTU9ERV9DT05O RUNUT1JfRFNJOwo+ICsKPiArCXJldHVybiAwOwo+ICt9Cj4gKwo+ICtzdGF0aWMgdm9pZCBkd19t aXBpX2RzaV9lbmNvZGVyX2Rpc2FibGUoc3RydWN0IGRybV9lbmNvZGVyICplbmNvZGVyKQo+ICt7 Cj4gKwlzdHJ1Y3QgZHdfbWlwaV9kc2lfcm9ja2NoaXAgKmRzaSA9IHRvX2RzaShlbmNvZGVyKTsK PiArCj4gKwlwbV9ydW50aW1lX3B1dChkc2ktPmRldik7Cj4gK30KPiArCj4gK3N0YXRpYyBjb25z dCBzdHJ1Y3QgZHJtX2VuY29kZXJfaGVscGVyX2Z1bmNzCj4gK2R3X21pcGlfZHNpX2VuY29kZXJf aGVscGVyX2Z1bmNzID0gewo+ICsJLm1vZGVfc2V0ID0gZHdfbWlwaV9kc2lfZW5jb2Rlcl9tb2Rl X3NldCwKPiArCS5hdG9taWNfY2hlY2sgPSBkd19taXBpX2RzaV9lbmNvZGVyX2F0b21pY19jaGVj aywKPiArCS5kaXNhYmxlID0gZHdfbWlwaV9kc2lfZW5jb2Rlcl9kaXNhYmxlLAo+ICt9OwoKLi4u IAoKQnJpYW4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K ZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752357AbdLFWBF (ORCPT ); Wed, 6 Dec 2017 17:01:05 -0500 Received: from mail-pg0-f65.google.com ([74.125.83.65]:37866 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752050AbdLFWBC (ORCPT ); Wed, 6 Dec 2017 17:01:02 -0500 X-Google-Smtp-Source: AGs4zMa4fFV0V7rwftEiTvcNwgs18MTm9nI9HVD8GKboUnLiBbUSPtw1KomUYNl7r6X4E9Cc2axaEQ== Date: Wed, 6 Dec 2017 13:52:43 -0800 From: Brian Norris To: Nickey Yang Cc: robh+dt@kernel.org, heiko@sntech.de, mark.rutland@arm.com, airlied@linux.ie, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, laurent.pinchart@ideasonboard.com, seanpaul@chromium.org, mka@chromium.org, hoegsberg@gmail.com, architt@codeaurora.org, philippe.cornu@st.com, yannick.fertre@st.com, hl@rock-chips.com, zyw@rock-chips.com, xbl@rock-chips.com Subject: Re: [PATCH v6 3/3] drm/rockchip: Add ROCKCHIP DW MIPI DSI controller driver Message-ID: <20171206215243.GB14257@google.com> References: <1512551301-12946-1-git-send-email-nickey.yang@rock-chips.com> <1512551301-12946-4-git-send-email-nickey.yang@rock-chips.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1512551301-12946-4-git-send-email-nickey.yang@rock-chips.com> 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 Hi Nickey, others, I just want to highlight a thing or two here. Otherwise, my 'Reviewed-by' still basically stands (FWIW). On Wed, Dec 06, 2017 at 05:08:21PM +0800, Nickey Yang wrote: > Add the ROCKCHIP DSI controller driver that uses the Synopsys DesignWare > MIPI DSI host controller bridge. > > Signed-off-by: Nickey Yang > Signed-off-by: Brian Norris > Reviewed-by: Brian Norris > Reviewed-by: Sean Paul > --- > change: > > v2: > add err_pllref, remove unnecessary encoder.enable & disable > correct spelling mistakes > v3: > call dw_mipi_dsi_unbind() in dw_mipi_dsi_rockchip_unbind() > fix typo, use of_device_get_match_data(), > change some ‘bind()’ logic into 'probe()' > add 'dev_set_drvdata()' > v4: > return -EINVAL when can not get best_freq > add a clarifying comment when get vco > add review tag > v5: > keep our power domain enabled while touching GRF > v6: > change func dw_mipi_encoder_disable name to > dw_mipi_dsi_encoder_disable We noticed a regression w.r.t. pm_runtime_*() handling using this patch, hence the pm_runtime changes in v5/v6. We actually need to keep our power domain enabled in the mode_set() function, where we start to configure some Rockchip-specific registers (GRF). More on that below. > > drivers/gpu/drm/rockchip/Kconfig | 2 +- > drivers/gpu/drm/rockchip/Makefile | 2 +- > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 1349 ----------------------- > drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c | 785 +++++++++++++ > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 +- > 6 files changed, 789 insertions(+), 1353 deletions(-) > delete mode 100644 drivers/gpu/drm/rockchip/dw-mipi-dsi.c > create mode 100644 drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c > ... > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c > new file mode 100644 > index 0000000..66ab6fe > --- /dev/null > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c > @@ -0,0 +1,785 @@ ... > +static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_display_mode *mode, > + struct drm_display_mode *adjusted) > +{ > + struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > + const struct rockchip_dw_dsi_chip_data *cdata = dsi->cdata; > + int val, ret, mux; > + > + mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, > + &dsi->encoder); > + if (mux < 0) > + return; > + /* > + * For the RK3399, the clk of grf must be enabled before writing grf > + * register. And for RK3288 or other soc, this grf_clk must be NULL, > + * the clk_prepare_enable return true directly. > + */ > + ret = clk_prepare_enable(dsi->grf_clk); > + if (ret) { > + DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret); > + return; > + } > + pm_runtime_get_sync(dsi->dev); What happens if there's a clk_prepare_enable() failure or failure to retrieve the endpoint ID earlier in this function? You won't call pm_runtime_get_*()...but might we still see a call to dw_mipi_dsi_encoder_disable(), which would mean we get unbalanced runtime PM status? Also (and more importantly), is it fair to do all of this in mode_set()? I believe Archit asked about this before, and the reason we're doing this stuff in mode_set() now (where previously, the Rockchip driver was doing it in ->enable()) is because when Philippe extracted the synopsys bridge driver, that code migrated to ->mode_set(). But, I'm reading the comments on drm_encoder_helper_funcs::mode_set, and I see: /** * @mode_set: * * This callback is used to update the display mode of an encoder. * * Note that the display pipe is completely off when this function is * called. Drivers which need hardware to be running before they program * the new display mode (because they implement runtime PM) should not * use this hook, because the helper library calls it only once and not * every time the display pipeline is suspend using either DPMS or the * new "ACTIVE" property. Such drivers should instead move all their * encoder setup into the @enable callback. That sounds like perhaps the synopsys driver should be moving to use ->enable() instead, so the Rockchip driver can do that as well? At any rate, I haven't found any real problem with using mode_set() so far, but I was just curious what the recommended practice was. > + val = cdata->dsi0_en_bit << 16; > + if (mux) > + val |= cdata->dsi0_en_bit; > + regmap_write(dsi->grf_regmap, cdata->grf_switch_reg, val); > + > + if (cdata->grf_dsi0_mode_reg) > + regmap_write(dsi->grf_regmap, cdata->grf_dsi0_mode_reg, > + cdata->grf_dsi0_mode); > + > + clk_disable_unprepare(dsi->grf_clk); > +} > + > +static int > +dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state); > + struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > + > + switch (dsi->format) { > + case MIPI_DSI_FMT_RGB888: > + s->output_mode = ROCKCHIP_OUT_MODE_P888; > + break; > + case MIPI_DSI_FMT_RGB666: > + s->output_mode = ROCKCHIP_OUT_MODE_P666; > + break; > + case MIPI_DSI_FMT_RGB565: > + s->output_mode = ROCKCHIP_OUT_MODE_P565; > + break; > + default: > + WARN_ON(1); > + return -EINVAL; > + } > + > + s->output_type = DRM_MODE_CONNECTOR_DSI; > + > + return 0; > +} > + > +static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder) > +{ > + struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > + > + pm_runtime_put(dsi->dev); > +} > + > +static const struct drm_encoder_helper_funcs > +dw_mipi_dsi_encoder_helper_funcs = { > + .mode_set = dw_mipi_dsi_encoder_mode_set, > + .atomic_check = dw_mipi_dsi_encoder_atomic_check, > + .disable = dw_mipi_dsi_encoder_disable, > +}; ... Brian