From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?ISO-8859-1?Q?St=FCbner?= Subject: Re: [PATCH v4 1/4] drm: rockchip: introduce rk3066 hdmi Date: Tue, 19 Mar 2019 12:44:03 +0100 Message-ID: <4125149.ebfS1HlHkS@diego> References: <20190306224113.24853-1-jbx6244@gmail.com> <20190306224113.24853-2-jbx6244@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20190306224113.24853-2-jbx6244@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Johan Jonker Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, airlied@linux.ie, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org SGkgSm9oYW4sCgpBbSBNaXR0d29jaCwgNi4gTcOkcnogMjAxOSwgMjM6NDE6MTAgQ0VUIHNjaHJp ZWIgSm9oYW4gSm9ua2VyOgo+IEZyb206IFpoZW5nIFlhbmcgPHpoZW5neWFuZ0Byb2NrLWNoaXBz LmNvbT4KPiAKPiBUaGUgUkszMDY2IEhETUkgVFggc2VydmVzIGFzIGludGVyZmFjZSBiZXR3ZWVu IGEgTENEIENvbnRyb2xsZXIgYW5kCj4gYSBIRE1JIGJ1cy4gQSBIRE1JIFRYIGNvbnNpc3RzIG9m IG9uZSBIRE1JIHRyYW5zbWl0dGVyIGNvbnRyb2xsZXIgYW5kCj4gb25lIEhETUkgdHJhbnNtaXR0 ZXIgUEhZLiBUaGUgaW50ZXJmYWNlIGhhcyB0aHJlZSAoMykgOC1iaXQgZGF0YSBjaGFubmVscwo+ IHdoaWNoIGNhbiBiZSBjb25maWd1cmVkIGZvciBhIG51bWJlciBvZiBidXMgd2lkdGhzICg4LzEw LzEyLzE2LzIwLzI0LWJpdCkKPiBhbmQgZGlmZmVyZW50IHZpZGVvIGZvcm1hdHMgKFJHQiwgWUNi Q3IpLgo+IAo+IEZlYXR1cmVzOgo+IEhETUkgdmVyc2lvbiAxLjRhLCBIRENQIHJldmlzaW9uIDEu NCBhbmQKPiBEVkkgdmVyc2lvbiAxLjAgY29tcGxpYW50IHRyYW5zbWl0dGVyLgo+IFN1cHBvcnRz IERUViByZXNvbHV0aW9ucyBmcm9tIDQ4MGkgdG8gMTA4MGkvcCBIRC4KPiBNYXN0ZXIgSTJDIGlu dGVyZmFjZSBmb3IgYSBEREMgY29ubmVjdGlvbi4KPiBIRE1JIFRYIHN1cHBvcnRzIG11bHRpcGxl IHBvd2VyIHNhdmUgbW9kZXMuCj4gVGhlIEhETUkgVFggaW5wdXQgY2FuIHN3aXRjaCBiZXR3ZWVu IExDREMwIGFuZCBMQ0RDMS4KPiAoU291bmQgc3VwcG9ydCBpcyBub3QgaW5jbHVkZWQgaW4gdGhp cyBwYXRjaCkKPiAKPiBTaWduZWQtb2ZmLWJ5OiBaaGVuZyBZYW5nIDx6aGVuZ3lhbmdAcm9jay1j aGlwcy5jb20+Cj4gU2lnbmVkLW9mZi1ieTogSm9oYW4gSm9ua2VyIDxqYng2MjQ0QGdtYWlsLmNv bT4KCkxvb2tzIGdvb2QgaW4gZ2VuZXJhbCwgYnV0IHRoZXJlIGFyZSBzb21lIG1pbm9yIHRoaW5n cyB0byBpbXByb3ZlIHlldCwKcGxlYXNlIHNlZSBiZWxvdyBmb3Igc3BlY2lmaWNzLgoKWy4uLl0K Cj4gK3N0YXRpYyB2b2lkIHJrMzA2Nl9oZG1pX2NvbmZpZ19waHkoc3RydWN0IHJrMzA2Nl9oZG1p ICpoZG1pKQo+ICt7Cj4gKwkvKiBUTURTIHVzZXMgdGhlIHNhbWUgZnJlcXVlbmN5IGFzIGRjbGsu ICovCj4gKwloZG1pX3dyaXRlYihoZG1pLCBIRE1JX0RFRVBfQ09MT1JfTU9ERSwgMHgyMik7CgpU aGVzZSBtYWdpYyB2YWx1ZXMgYmVsb3cgYXJlIG5vIGZhdWx0IG9mIHlvdXJzLCBidXQgaW4gYW55 IGNhc2UgdGhpcwpjb3VsZCB1c2UgYSBjb21tZW50IHRoYXQgdGhlIHNlbWktcHVibGljIGRvY3Vt ZW50YXRpb24gZG9lcyBub3QKZGVzY3JpYmUgaGRtaS1yZWdpc3RlcnMgYXQgYWxsLCBzbyB3ZSdy ZSBzdHVjayB3aXRoIHRoZXNlIG1hZ2ljLXZhbHVlcwpmb3Igbm93LgoKPiArCWlmIChoZG1pLT50 bWRzY2xrID4gMTAwMDAwMDAwKSB7Cj4gKwkJcmszMDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4 MTU4LCAweDBFKTsKPiArCQlyazMwNjZfaGRtaV9waHlfd3JpdGUoaGRtaSwgMHgxNWMsIDB4MDAp Owo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZShoZG1pLCAweDE2MCwgMHg2MCk7Cj4gKwkJcmsz MDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTY0LCAweDAwKTsKPiArCQlyazMwNjZfaGRtaV9w aHlfd3JpdGUoaGRtaSwgMHgxNjgsIDB4REEpOwo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZSho ZG1pLCAweDE2YywgMHhBMSk7Cj4gKwkJcmszMDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTcw LCAweDBlKTsKPiArCQlyazMwNjZfaGRtaV9waHlfd3JpdGUoaGRtaSwgMHgxNzQsIDB4MjIpOwo+ ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZShoZG1pLCAweDE3OCwgMHgwMCk7Cj4gKwl9IGVsc2Ug aWYgKGhkbWktPnRtZHNjbGsgPiA1MDAwMDAwMCkgewo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0 ZShoZG1pLCAweDE1OCwgMHgwNik7Cj4gKwkJcmszMDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4 MTVjLCAweDAwKTsKPiArCQlyazMwNjZfaGRtaV9waHlfd3JpdGUoaGRtaSwgMHgxNjAsIDB4NjAp Owo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZShoZG1pLCAweDE2NCwgMHgwMCk7Cj4gKwkJcmsz MDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTY4LCAweENBKTsKPiArCQlyazMwNjZfaGRtaV9w aHlfd3JpdGUoaGRtaSwgMHgxNmMsIDB4QTMpOwo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZSho ZG1pLCAweDE3MCwgMHgwZSk7Cj4gKwkJcmszMDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTc0 LCAweDIwKTsKPiArCQlyazMwNjZfaGRtaV9waHlfd3JpdGUoaGRtaSwgMHgxNzgsIDB4MDApOwo+ ICsJfSBlbHNlIHsKPiArCQlyazMwNjZfaGRtaV9waHlfd3JpdGUoaGRtaSwgMHgxNTgsIDB4MDIp Owo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZShoZG1pLCAweDE1YywgMHgwMCk7Cj4gKwkJcmsz MDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTYwLCAweDYwKTsKPiArCQlyazMwNjZfaGRtaV9w aHlfd3JpdGUoaGRtaSwgMHgxNjQsIDB4MDApOwo+ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZSho ZG1pLCAweDE2OCwgMHhDMik7Cj4gKwkJcmszMDY2X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTZj LCAweEEyKTsKPiArCQlyazMwNjZfaGRtaV9waHlfd3JpdGUoaGRtaSwgMHgxNzAsIDB4MGUpOwo+ ICsJCXJrMzA2Nl9oZG1pX3BoeV93cml0ZShoZG1pLCAweDE3NCwgMHgyMCk7Cj4gKwkJcmszMDY2 X2hkbWlfcGh5X3dyaXRlKGhkbWksIDB4MTc4LCAweDAwKTsKPiArCX0KPiArfQoKPiArc3RhdGlj IHZvaWQgcmszMDY2X2hkbWlfZW5jb2Rlcl9lbmFibGUoc3RydWN0IGRybV9lbmNvZGVyICplbmNv ZGVyKQo+ICt7Cj4gKwlzdHJ1Y3QgcmszMDY2X2hkbWkgKmhkbWkgPSB0b19yazMwNjZfaGRtaShl bmNvZGVyKTsKPiArCWludCBtdXgsIHZhbDsKPiArCj4gKwltdXggPSBkcm1fb2ZfZW5jb2Rlcl9h Y3RpdmVfZW5kcG9pbnRfaWQoaGRtaS0+ZGV2LT5vZl9ub2RlLCBlbmNvZGVyKTsKPiArCWlmICht dXgpCj4gKwkJdmFsID0gQklUKDMwKSB8IEJJVCgxNCk7Cj4gKwllbHNlCj4gKwkJdmFsID0gQklU KDMwKTsKPiArCj4gKwlyZWdtYXBfd3JpdGUoaGRtaS0+Z3JmLCAweDE1MCwgdmFsKTsKCgpQbGVh c2UgZGVmaW5lIGNvbnN0YW50cyBmb3IgYm90aCBCSVQoMTQpIGFuZCB0aGUgMHgxNTAgR1JGIHJl Z2lzdGVyLAp3aGljaCBpcyBHUkZfU09DX0NPTjAsIHNvIGluIHRoZSBoZWFkZXIKCiNkZWZpbmUg R1JGX1NPQ19DT04wCTB4MTUwCiNkZWZpbmUgSERNSV9WSURFT19TRUwJQklUKDE0KQoKYW5kIHRo ZW4gZG8KCWlmIChtdXgpCgkJdmFsID0gKEhETUlfVklERU9fU0VMIDw8IDE2KSB8IEhETUlfVklE RU9fU0VMOwoJZWxzZQoJCXZhbCA9IEhETUlfVklERU9fU0VMIDw8IDE2OwoKCXJlZ21hcF93cml0 ZShoZG1pLT5ncmYsIEdSRl9TT0NfQ09OMCwgdmFsKTsKCj4gKwo+ICsJZGV2X2RiZyhoZG1pLT5k ZXYsICJoZG1pIGVuY29kZXIgZW5hYmxlIHNlbGVjdDogdm9wJXNcbiIsCj4gKwkJKG11eCkgPyAi MSIgOiAiMCIpOwo+ICsKPiArCXJrMzA2Nl9oZG1pX3NldHVwKGhkbWksICZoZG1pLT5wcmV2aW91 c19tb2RlKTsKPiArfQo+ICsKCgo+ICtzdGF0aWMgY29uc3Qgc3RydWN0IGRybV9kaXNwbGF5X21v ZGUgZWRpZF9jZWFfbW9kZXNbXSA9IHsKPiArCS8qIDQgLSAxMjgweDcyMEA2MEh6IDE2OjkgKi8K PiArCXsgRFJNX01PREUoIjEyODB4NzIwIiwgRFJNX01PREVfVFlQRV9EUklWRVIsIDc0MjUwLCAx MjgwLCAxMzkwLAo+ICsJCSAgIDE0MzAsIDE2NTAsIDAsIDcyMCwgNzI1LCA3MzAsIDc1MCwgMCwK PiArCQkgICBEUk1fTU9ERV9GTEFHX1BIU1lOQyB8IERSTV9NT0RFX0ZMQUdfUFZTWU5DKSwKPiAr CSAgLnZyZWZyZXNoID0gNjAsIC5waWN0dXJlX2FzcGVjdF9yYXRpbyA9IEhETUlfUElDVFVSRV9B U1BFQ1RfMTZfOSwgfSwKPiArfTsKCnBsZWFzZSBkcm9wIHRoYXQsIHNlZSBiZWxvdwoKPiArc3Rh dGljIGludCByazMwNjZfaGRtaV9jb25uZWN0b3JfZ2V0X21vZGVzKHN0cnVjdCBkcm1fY29ubmVj dG9yICpjb25uZWN0b3IpCj4gK3sKPiArCXN0cnVjdCByazMwNjZfaGRtaSAqaGRtaSA9IHRvX3Jr MzA2Nl9oZG1pKGNvbm5lY3Rvcik7Cj4gKwlzdHJ1Y3QgZHJtX2Rpc3BsYXlfbW9kZSAqbW9kZSA9 IE5VTEw7Cj4gKwlzdHJ1Y3QgZWRpZCAqZWRpZDsKPiArCWludCByZXQgPSAwOwo+ICsKPiArCWlm ICghaGRtaS0+ZGRjKQo+ICsJCXJldHVybiAwOwo+ICsKPiArCWhkbWktPmhkbWlfZGF0YS5zaW5r X2lzX2hkbWkgPSBmYWxzZTsKPiArCj4gKwllZGlkID0gZHJtX2dldF9lZGlkKGNvbm5lY3Rvciwg aGRtaS0+ZGRjKTsKPiArCWlmIChlZGlkKSB7Cj4gKwkJaGRtaS0+aGRtaV9kYXRhLnNpbmtfaXNf aGRtaSA9IGRybV9kZXRlY3RfaGRtaV9tb25pdG9yKGVkaWQpOwo+ICsKPiArCQlkZXZfaW5mbyho ZG1pLT5kZXYsICJtb25pdG9yIHR5cGUgOiAlcyA6ICVkeCVkIGNtXG4iLAo+ICsJCQkoaGRtaS0+ aGRtaV9kYXRhLnNpbmtfaXNfaGRtaSA/ICJIRE1JIiA6ICJEVkkiKSwKPiArCQkJZWRpZC0+d2lk dGhfY20sIGVkaWQtPmhlaWdodF9jbSk7Cj4gKwo+ICsJCWRybV9jb25uZWN0b3JfdXBkYXRlX2Vk aWRfcHJvcGVydHkoY29ubmVjdG9yLCBlZGlkKTsKPiArCQlyZXQgPSBkcm1fYWRkX2VkaWRfbW9k ZXMoY29ubmVjdG9yLCBlZGlkKTsKPiArCQlrZnJlZShlZGlkKTsKPiArCX0KPiArCj4gKwlpZiAo KHJldCA9PSAwKSB8fCAoaGRtaS0+aGRtaV9kYXRhLnNpbmtfaXNfaGRtaSA9PSBmYWxzZSkpIHsK PiArCQloZG1pLT5oZG1pX2RhdGEuc2lua19pc19oZG1pID0gZmFsc2U7Cj4gKwo+ICsJCW1vZGUg PSBkcm1fbW9kZV9kdXBsaWNhdGUoaGRtaS0+ZHJtX2RldiwgJmVkaWRfY2VhX21vZGVzWzBdKTsK PiArCQlpZiAoIW1vZGUpCj4gKwkJCXJldHVybiByZXQ7Cj4gKwkJbW9kZS0+dHlwZSB8PSBEUk1f TU9ERV9UWVBFX1BSRUZFUlJFRDsKPiArCQlkcm1fbW9kZV9wcm9iZWRfYWRkKGNvbm5lY3Rvciwg bW9kZSk7Cj4gKwkJcmV0Kys7Cj4gKwo+ICsJCWRldl9pbmZvKGhkbWktPmRldiwgIm5vIENFQSBt b2RlIGZvdW5kLCB1c2UgZGVmYXVsdFxuIik7CgpUaGlzIGlzIGEgaGFjayBmcm9tIHRoZSB2ZW5k b3Ita2VybmVsLgoKSWYgRURJRCByZWFkaW5nIGZhaWxzLCBpdCBpcyBzb21lIHNvcnQgb2YgaXNz dWUgd2l0aCB0aGUgZHJpdmVyLCBzbyB3ZQpzaG91bGRuJ3QgYXNzdW1lIGFueSBzcGVjaWZpYyBt b2RlIGF0IGFsbC4gU2VlIHRoZSBzb21ld2hhdCBzaW1pbGFyCmlubm9faGRtaSBkcml2ZXIgZm9y IGNvbXBhcmlzb24uCgoKPiArCX0KPiArCj4gKwlyZXR1cm4gcmV0Owo+ICt9Cj4gKwoKCgo+ICtz dGF0aWMgaW50Cj4gK3JrMzA2Nl9oZG1pX3JlZ2lzdGVyKHN0cnVjdCBkcm1fZGV2aWNlICpkcm0s IHN0cnVjdCByazMwNjZfaGRtaSAqaGRtaSkKPiArewo+ICsJc3RydWN0IGRybV9lbmNvZGVyICpl bmNvZGVyID0gJmhkbWktPmVuY29kZXI7Cj4gKwlzdHJ1Y3QgZGV2aWNlICpkZXYgPSBoZG1pLT5k ZXY7Cj4gKwo+ICsJZW5jb2Rlci0+cG9zc2libGVfY3J0Y3MgPQo+ICsJCWRybV9vZl9maW5kX3Bv c3NpYmxlX2NydGNzKGRybSwgZGV2LT5vZl9ub2RlKTsKPiArCj4gKwkvKgo+ICsJICogSWYgd2Ug ZmFpbGVkIHRvIGZpbmQgdGhlIFZPUChzKSB3aGljaCB0aGlzIGVuY29kZXIgaXMKPiArCSAqIHN1 cHBvc2VkIHRvIGJlIGNvbm5lY3RlZCB0bywgaXQncyBiZWNhdXNlIHRoZSBWT1AgaGFzCj4gKwkg KiBub3QgYmVlbiByZWdpc3RlcmVkIHlldC4gIERlZmVyIHByb2JpbmcsIGFuZCBob3BlIHRoYXQK PiArCSAqIHRoZSByZXF1aXJlZCBWT1AgaXMgYWRkZWQgbGF0ZXIuCj4gKwkgKi8KPiArCWlmIChl bmNvZGVyLT5wb3NzaWJsZV9jcnRjcyA9PSAwKQo+ICsJCXJldHVybiAtRVBST0JFX0RFRkVSOwo+ ICsKPiArCWRldl9pbmZvKGhkbWktPmRldiwgIlZPUCBmb3VuZFxuIik7Cgp1bm5lY2Vzc2FyeSBv dXRwdXQgd2hpY2ggd2lsbCBjbHV0dGVyIHVwIHRoZSBrZXJuZWwgbG9nLCBwbGVhc2UgZHJvcC4K Cj4gKwo+ICsJZHJtX2VuY29kZXJfaGVscGVyX2FkZChlbmNvZGVyLCAmcmszMDY2X2hkbWlfZW5j b2Rlcl9oZWxwZXJfZnVuY3MpOwo+ICsJZHJtX2VuY29kZXJfaW5pdChkcm0sIGVuY29kZXIsICZy azMwNjZfaGRtaV9lbmNvZGVyX2Z1bmNzLAo+ICsJCQkgRFJNX01PREVfRU5DT0RFUl9UTURTLCBO VUxMKTsKPiArCj4gKwloZG1pLT5jb25uZWN0b3IucG9sbGVkID0gRFJNX0NPTk5FQ1RPUl9QT0xM X0hQRDsKPiArCj4gKwlkcm1fY29ubmVjdG9yX2hlbHBlcl9hZGQoJmhkbWktPmNvbm5lY3RvciwK PiArCQkJCSAmcmszMDY2X2hkbWlfY29ubmVjdG9yX2hlbHBlcl9mdW5jcyk7Cj4gKwlkcm1fY29u bmVjdG9yX2luaXQoZHJtLCAmaGRtaS0+Y29ubmVjdG9yLAo+ICsJCQkgICAmcmszMDY2X2hkbWlf Y29ubmVjdG9yX2Z1bmNzLAo+ICsJCQkgICBEUk1fTU9ERV9DT05ORUNUT1JfSERNSUEpOwo+ICsK PiArCWRybV9jb25uZWN0b3JfYXR0YWNoX2VuY29kZXIoJmhkbWktPmNvbm5lY3RvciwgZW5jb2Rl cik7Cj4gKwo+ICsJcmV0dXJuIDA7Cj4gK30KPiArCj4gK3N0YXRpYyBpcnFyZXR1cm5fdCByazMw NjZfaGRtaV9pMmNfaXJxKHN0cnVjdCByazMwNjZfaGRtaSAqaGRtaSwgdTggc3RhdCkKPiArewo+ ICsJc3RydWN0IHJrMzA2Nl9oZG1pX2kyYyAqaTJjID0gaGRtaS0+aTJjOwo+ICsKPiArCWlmICgh KHN0YXQgJiBIRE1JX0lOVFJfRURJRF9NQVNLKSkKPiArCQlyZXR1cm4gSVJRX05PTkU7Cj4gKwo+ ICsJaTJjLT5zdGF0ID0gc3RhdDsKPiArCj4gKwljb21wbGV0ZSgmaTJjLT5jb21wbCk7Cj4gKwo+ ICsJcmV0dXJuIElSUV9IQU5ETEVEOwo+ICt9Cj4gKwo+ICtzdGF0aWMgaXJxcmV0dXJuX3Qgcmsz MDY2X2hkbWlfaGFyZGlycShpbnQgaXJxLCB2b2lkICpkZXZfaWQpCj4gK3sKPiArCXN0cnVjdCBy azMwNjZfaGRtaSAqaGRtaSA9IGRldl9pZDsKPiArCWlycXJldHVybl90IHJldCA9IElSUV9OT05F Owo+ICsJdTggaW50ZXJydXB0Owo+ICsKPiArCWlmIChyazMwNjZfaGRtaV9nZXRfcG93ZXJfbW9k ZShoZG1pKSA9PSBIRE1JX1NZU19QT1dFUl9NT0RFX0EpCj4gKwkJaGRtaV93cml0ZWIoaGRtaSwg SERNSV9TWVNfQ1RSTCwgSERNSV9TWVNfUE9XRVJfTU9ERV9CKTsKPiArCj4gKwlpbnRlcnJ1cHQg PSBoZG1pX3JlYWRiKGhkbWksIEhETUlfSU5UUl9TVEFUVVMxKTsKPiArCWlmIChpbnRlcnJ1cHQp Cj4gKwkJaGRtaV93cml0ZWIoaGRtaSwgSERNSV9JTlRSX1NUQVRVUzEsIGludGVycnVwdCk7Cj4g Kwo+ICsJaWYgKGhkbWktPmkyYykKPiArCQlyZXQgPSByazMwNjZfaGRtaV9pMmNfaXJxKGhkbWks IGludGVycnVwdCk7CgpJIHRoaW5rIHlvdSBkb24ndCByZWFsbHkgbmVlZCB0aGF0IHJrMzA2Nl9o ZG1pX2kyY19pcnEgZnVuY3Rpb24gYWJvdmUsCnRoaXMgY2FuIGp1c3QgYmVjb21lCgoJaWYgKGlu dGVycnVwdCAmIEhETUlfSU5UUl9FRElEX01BU0spIHsKCQloZG1pLT5pMmMtPnN0YXQgPSBzdGF0 OwoJCWNvbXBsZXRlKCZoZG1pLT5pMmMtPmNvbXBsKTsKCX0KCmhkbWktPmkyYyBpcyBzZXQgdGhy b3VnaCByazMwNjZfaGRtaV9pMmNfYWRhcHRlcigpIHdoaWNoIGlzIGFsd2F5cyBjYWxsZWQKYW5k IG5lZWRzIHRvIGJlIHN1Y2Vzc2Z1bCBiZWZvcmUgcmVnaXN0ZXJpbmcgdGhlIGludGVycnVwdCwg c28gdGhlcmUgaXMKbm8gbmVlZCB0byBjaGVjayBmb3IgaGRtaS0+aTJjIGhlcmUgYXQgYWxsLgoK Cj4gKwo+ICsJaWYgKGludGVycnVwdCAmIChIRE1JX0lOVFJfSE9UUExVRyB8IEhETUlfSU5UUl9N U0VOUykpCj4gKwkJcmV0ID0gSVJRX1dBS0VfVEhSRUFEOwo+ICsKPiArCXJldHVybiByZXQ7Cj4g K30KPiArCgoKCgoKPiArc3RhdGljIHN0cnVjdCBpMmNfYWRhcHRlciAqcmszMDY2X2hkbWlfaTJj X2FkYXB0ZXIoc3RydWN0IHJrMzA2Nl9oZG1pICpoZG1pKQo+ICt7Cj4gKwlzdHJ1Y3QgaTJjX2Fk YXB0ZXIgKmFkYXA7Cj4gKwlzdHJ1Y3QgcmszMDY2X2hkbWlfaTJjICppMmM7Cj4gKwlpbnQgcmV0 Owo+ICsKPiArCWkyYyA9IGRldm1fa3phbGxvYyhoZG1pLT5kZXYsIHNpemVvZigqaTJjKSwgR0ZQ X0tFUk5FTCk7Cj4gKwlpZiAoIWkyYykKPiArCQlyZXR1cm4gRVJSX1BUUigtRU5PTUVNKTsKPiAr Cj4gKwltdXRleF9pbml0KCZpMmMtPmkyY19sb2NrKTsKPiArCWluaXRfY29tcGxldGlvbigmaTJj LT5jb21wbCk7Cj4gKwo+ICsJYWRhcCA9ICZpMmMtPmFkYXA7Cj4gKwlhZGFwLT5jbGFzcyA9IEky Q19DTEFTU19EREM7Cj4gKwlhZGFwLT5vd25lciA9IFRISVNfTU9EVUxFOwo+ICsJYWRhcC0+ZGV2 LnBhcmVudCA9IGhkbWktPmRldjsKPiArCWFkYXAtPmRldi5vZl9ub2RlID0gaGRtaS0+ZGV2LT5v Zl9ub2RlOwo+ICsJYWRhcC0+YWxnbyA9ICZyazMwNjZfaGRtaV9hbGdvcml0aG07Cj4gKwlzdHJs Y3B5KGFkYXAtPm5hbWUsICJSSzMwNjYgSERNSSIsIHNpemVvZihhZGFwLT5uYW1lKSk7Cj4gKwlp MmNfc2V0X2FkYXBkYXRhKGFkYXAsIGhkbWkpOwo+ICsKPiArCXJldCA9IGkyY19hZGRfYWRhcHRl cihhZGFwKTsKPiArCWlmIChyZXQpIHsKPiArCQlkZXZfd2FybihoZG1pLT5kZXYsICJjYW5ub3Qg YWRkICVzIEkyQyBhZGFwdGVyXG4iLCBhZGFwLT5uYW1lKTsKPiArCQlkZXZtX2tmcmVlKGhkbWkt PmRldiwgaTJjKTsKPiArCQlyZXR1cm4gRVJSX1BUUihyZXQpOwo+ICsJfQo+ICsKPiArCWhkbWkt PmkyYyA9IGkyYzsKPiArCj4gKwlkZXZfaW5mbyhoZG1pLT5kZXYsICJyZWdpc3RlcmVkICVzIEky QyBidXMgZHJpdmVyXG4iLCBhZGFwLT5uYW1lKTsKClBsZWFzZSBkcm9wIG9yIG1ha2UgaXQgYSBE Uk1fREVWX0RFQlVHLiBBbHNvLCBwbGVhc2UgY29udmVydCBhbGwgdGhlCmdlbmVyYWwgZGV2X2Zv b2JhciBjYWxscyB0byB0aGVpciBEUk1fKiBlcXVpdmFsZW50cywgc28KZGV2X3dhcm4gYmVjb21l cyBEUk1fREVWX0VSUk9SLCBkZXZfaW5mbyBEUk1fREVWX0lORk8gYW5kIHNvIG9uLgoKCj4gKwly ZXR1cm4gYWRhcDsKPiArfQo+ICsKPiArc3RhdGljIGludCByazMwNjZfaGRtaV9iaW5kKHN0cnVj dCBkZXZpY2UgKmRldiwgc3RydWN0IGRldmljZSAqbWFzdGVyLAo+ICsJCQkgICAgdm9pZCAqZGF0 YSkKPiArewo+ICsJc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldiA9IHRvX3BsYXRmb3JtX2Rl dmljZShkZXYpOwo+ICsJc3RydWN0IGRybV9kZXZpY2UgKmRybSA9IGRhdGE7Cj4gKwlzdHJ1Y3Qg cmszMDY2X2hkbWkgKmhkbWk7Cj4gKwlzdHJ1Y3QgcmVzb3VyY2UgKmlvcmVzOwo+ICsJaW50IGly cTsKPiArCWludCByZXQ7Cj4gKwo+ICsJaGRtaSA9IGRldm1fa3phbGxvYyhkZXYsIHNpemVvZigq aGRtaSksIEdGUF9LRVJORUwpOwo+ICsJaWYgKCFoZG1pKQo+ICsJCXJldHVybiAtRU5PTUVNOwo+ ICsKPiArCWhkbWktPmRldiA9IGRldjsKPiArCWhkbWktPmRybV9kZXYgPSBkcm07Cj4gKwo+ICsJ aW9yZXMgPSBwbGF0Zm9ybV9nZXRfcmVzb3VyY2UocGRldiwgSU9SRVNPVVJDRV9NRU0sIDApOwo+ ICsJaWYgKCFpb3JlcykKPiArCQlyZXR1cm4gLUVOWElPOwo+ICsKPiArCWhkbWktPnJlZ3MgPSBk ZXZtX2lvcmVtYXBfcmVzb3VyY2UoZGV2LCBpb3Jlcyk7Cj4gKwlpZiAoSVNfRVJSKGhkbWktPnJl Z3MpKQo+ICsJCXJldHVybiBQVFJfRVJSKGhkbWktPnJlZ3MpOwo+ICsKPiArCWlycSA9IHBsYXRm b3JtX2dldF9pcnEocGRldiwgMCk7Cj4gKwlpZiAoaXJxIDwgMCkKPiArCQlyZXR1cm4gaXJxOwo+ ICsKPiArCWhkbWktPmhjbGsgPSBkZXZtX2Nsa19nZXQoZGV2LCAiaGNsayIpOwo+ICsJaWYgKElT X0VSUihoZG1pLT5oY2xrKSkgewo+ICsJCWRldl9lcnIoZGV2LCAidW5hYmxlIHRvIGdldCBIRE1J IGhjbGsgY2xvY2tcbiIpOwo+ICsJCXJldHVybiBQVFJfRVJSKGhkbWktPmhjbGspOwo+ICsJfQo+ ICsKPiArCXJldCA9IGNsa19wcmVwYXJlX2VuYWJsZShoZG1pLT5oY2xrKTsKPiArCWlmIChyZXQp IHsKPiArCQlkZXZfZXJyKGRldiwgImNhbm5vdCBlbmFibGUgSERNSSBoY2xrIGNsb2NrOiAlZFxu IiwgcmV0KTsKPiArCQlyZXR1cm4gcmV0Owo+ICsJfQo+ICsKPiArCWhkbWktPmdyZiA9IHN5c2Nv bl9yZWdtYXBfbG9va3VwX2J5X3BoYW5kbGUoZGV2LT5vZl9ub2RlLAo+ICsJCQkJCQkgICAgInJv Y2tjaGlwLGdyZiIpOwo+ICsJaWYgKElTX0VSUihoZG1pLT5ncmYpKSB7Cj4gKwkJZGV2X2Vycihk ZXYsICJ1bmFibGUgdG8gZ2V0IHJvY2tjaGlwLGdyZlxuIik7Cj4gKwkJcmV0ID0gUFRSX0VSUiho ZG1pLT5ncmYpOwo+ICsJCWdvdG8gZXJyX2Rpc2FibGVfaGNsazsKPiArCX0KPiArCj4gKwkvKiBp bnRlcm5hbCBoY2xrID0gaGRtaV9oY2xrIC8gMjUgKi8KPiArCWhkbWlfd3JpdGViKGhkbWksIEhE TUlfSU5URVJOQUxfQ0xLX0RJVklERVIsIDI1KTsKPiArCj4gKwloZG1pLT5kZGMgPSByazMwNjZf aGRtaV9pMmNfYWRhcHRlcihoZG1pKTsKPiArCWlmIChJU19FUlIoaGRtaS0+ZGRjKSkgewo+ICsJ CXJldCA9IFBUUl9FUlIoaGRtaS0+ZGRjKTsKPiArCQloZG1pLT5kZGMgPSBOVUxMOwo+ICsJCWdv dG8gZXJyX2Rpc2FibGVfaGNsazsKPiArCX0KPiArCj4gKwlyazMwNjZfaGRtaV9zZXRfcG93ZXJf bW9kZShoZG1pLCBIRE1JX1NZU19QT1dFUl9NT0RFX0IpOwo+ICsJdXNsZWVwX3JhbmdlKDk5OSwg MTAwMCk7Cj4gKwloZG1pX3dyaXRlYihoZG1pLCBIRE1JX0lOVFJfTUFTSzEsIEhETUlfSU5UUl9I T1RQTFVHKTsKPiArCWhkbWlfd3JpdGViKGhkbWksIEhETUlfSU5UUl9NQVNLMiwgMCk7Cj4gKwlo ZG1pX3dyaXRlYihoZG1pLCBIRE1JX0lOVFJfTUFTSzMsIDApOwo+ICsJaGRtaV93cml0ZWIoaGRt aSwgSERNSV9JTlRSX01BU0s0LCAwKTsKPiArCXJrMzA2Nl9oZG1pX3NldF9wb3dlcl9tb2RlKGhk bWksIEhETUlfU1lTX1BPV0VSX01PREVfQSk7Cj4gKwo+ICsJcmV0ID0gcmszMDY2X2hkbWlfcmVn aXN0ZXIoZHJtLCBoZG1pKTsKPiArCWlmIChyZXQpCj4gKwkJZ290byBlcnJfZGlzYWJsZV9oY2xr OwoKZ290byBlcnJfZGlzYWJsZV9pMmM7CgpTbyB0aGF0IHRoZSBpMmMtYWRhcHRlciBhbHNvIGdl dHMgZGlzYWJsZWQgb24gZXJyb3IuCgo+ICsJZGV2X3NldF9kcnZkYXRhKGRldiwgaGRtaSk7Cj4g Kwo+ICsJcmV0ID0gZGV2bV9yZXF1ZXN0X3RocmVhZGVkX2lycShkZXYsIGlycSwgcmszMDY2X2hk bWlfaGFyZGlycSwKPiArCQkJCQlyazMwNjZfaGRtaV9pcnEsIElSUUZfU0hBUkVELAo+ICsJCQkJ CWRldl9uYW1lKGRldiksIGhkbWkpOwo+ICsJaWYgKHJldCkgewo+ICsJCWRldl9lcnIoZGV2LCAi ZmFpbGVkIHRvIHJlcXVlc3QgaGRtaSBpcnE6ICVkXG4iLCByZXQpOwo+ICsJCWdvdG8gZXJyX2Rp c2FibGVfaGNsazsKCmdvdG8gZXJyX2Rpc2FibGVfaTJjOwoKPiArCX0KPiArCj4gKwlyZXR1cm4g MDsKPiArCgplcnJfZGlzYWJsZV9pMmM6CglpMmNfcHV0X2FkYXB0ZXIoaGRtaS0+ZGRjKTsKCj4g K2Vycl9kaXNhYmxlX2hjbGs6Cj4gKwljbGtfZGlzYWJsZV91bnByZXBhcmUoaGRtaS0+aGNsayk7 Cj4gKwo+ICsJcmV0dXJuIHJldDsKPiArfQo+ICsKPiArc3RhdGljIHZvaWQgcmszMDY2X2hkbWlf dW5iaW5kKHN0cnVjdCBkZXZpY2UgKmRldiwgc3RydWN0IGRldmljZSAqbWFzdGVyLAo+ICsJCQkg ICAgICAgdm9pZCAqZGF0YSkKPiArewo+ICsJc3RydWN0IHJrMzA2Nl9oZG1pICpoZG1pID0gZGV2 X2dldF9kcnZkYXRhKGRldik7Cj4gKwo+ICsJaGRtaS0+Y29ubmVjdG9yLmZ1bmNzLT5kZXN0cm95 KCZoZG1pLT5jb25uZWN0b3IpOwo+ICsJaGRtaS0+ZW5jb2Rlci5mdW5jcy0+ZGVzdHJveSgmaGRt aS0+ZW5jb2Rlcik7Cj4gKwo+ICsJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGhkbWktPmhjbGspOwo+ ICsJaTJjX3B1dF9hZGFwdGVyKGhkbWktPmRkYyk7Cgp5b3Ugc2hvdWxkIHByb2JhYmx5IGludmVy dCB0aGUgY2FsbGluZyBvcmRlciBoZXJlLCBmaXJzdCByZW1vdmUgdGhlCmkyYyBhZGFwdGVyIGFu ZCBvbmx5IHRoZW4gZGlzYWJsZSB0aGUgY2xvY2suCgoKVGhhbmtzCkhlaWtvCgoKX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcg bGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVs From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 211C5C4360F for ; Tue, 19 Mar 2019 11:44:27 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E231220857 for ; Tue, 19 Mar 2019 11:44:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="C9xjXnph" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E231220857 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sntech.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3IgKQeuqWBo3smJ14/QoFnyN3a188wi8FXRT6JWZmAU=; b=C9xjXnph0iEWNb r7fFiFqqUJR3hWPLBCcJpMWV/PhuqLQPiwV7bEcdKNgrkSxivJIl8zlxvyqX3Zk+1fxruFk/ARbsL EILY/rvykLcV0MOh6npsEupcPaFuM3zi91hu2FXJyXydHe2w3EOdBAXJaNf81zFlSlbmUMixGGGjM dZkDBiyXRF0kTa1TT1XCUxb3YhBR1Sxl2f1Bf9oUhufv7pMGU0i5RPXkAZwlC+kin4wHU6y2XN/9g QKFjD7j5QcR+hfilc0PJHWQBnkFUnCicIjXdYlVQiNmsEw2/mobQJ+oyGyJbkEuCc4+omNAAHT3nw KODjl+1tCK5op8L186Hg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6DAK-0001hh-85; Tue, 19 Mar 2019 11:44:20 +0000 Received: from gloria.sntech.de ([185.11.138.130]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h6DAG-0001hH-S7; Tue, 19 Mar 2019 11:44:18 +0000 Received: from ip5f5a6320.dynamic.kabel-deutschland.de ([95.90.99.32] helo=diego.localnet) by gloria.sntech.de with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1h6DA4-0006B0-Ec; Tue, 19 Mar 2019 12:44:04 +0100 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Johan Jonker Subject: Re: [PATCH v4 1/4] drm: rockchip: introduce rk3066 hdmi Date: Tue, 19 Mar 2019 12:44:03 +0100 Message-ID: <4125149.ebfS1HlHkS@diego> In-Reply-To: <20190306224113.24853-2-jbx6244@gmail.com> References: <20190306224113.24853-1-jbx6244@gmail.com> <20190306224113.24853-2-jbx6244@gmail.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190319_044417_060576_015F984C X-CRM114-Status: GOOD ( 25.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, airlied@linux.ie, hjc@rock-chips.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, robh+dt@kernel.org, daniel@ffwll.ch, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Johan, Am Mittwoch, 6. M=E4rz 2019, 23:41:10 CET schrieb Johan Jonker: > From: Zheng Yang > = > The RK3066 HDMI TX serves as interface between a LCD Controller and > a HDMI bus. A HDMI TX consists of one HDMI transmitter controller and > one HDMI transmitter PHY. The interface has three (3) 8-bit data channels > which can be configured for a number of bus widths (8/10/12/16/20/24-bit) > and different video formats (RGB, YCbCr). > = > Features: > HDMI version 1.4a, HDCP revision 1.4 and > DVI version 1.0 compliant transmitter. > Supports DTV resolutions from 480i to 1080i/p HD. > Master I2C interface for a DDC connection. > HDMI TX supports multiple power save modes. > The HDMI TX input can switch between LCDC0 and LCDC1. > (Sound support is not included in this patch) > = > Signed-off-by: Zheng Yang > Signed-off-by: Johan Jonker Looks good in general, but there are some minor things to improve yet, please see below for specifics. [...] > +static void rk3066_hdmi_config_phy(struct rk3066_hdmi *hdmi) > +{ > + /* TMDS uses the same frequency as dclk. */ > + hdmi_writeb(hdmi, HDMI_DEEP_COLOR_MODE, 0x22); These magic values below are no fault of yours, but in any case this could use a comment that the semi-public documentation does not describe hdmi-registers at all, so we're stuck with these magic-values for now. > + if (hdmi->tmdsclk > 100000000) { > + rk3066_hdmi_phy_write(hdmi, 0x158, 0x0E); > + rk3066_hdmi_phy_write(hdmi, 0x15c, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x160, 0x60); > + rk3066_hdmi_phy_write(hdmi, 0x164, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x168, 0xDA); > + rk3066_hdmi_phy_write(hdmi, 0x16c, 0xA1); > + rk3066_hdmi_phy_write(hdmi, 0x170, 0x0e); > + rk3066_hdmi_phy_write(hdmi, 0x174, 0x22); > + rk3066_hdmi_phy_write(hdmi, 0x178, 0x00); > + } else if (hdmi->tmdsclk > 50000000) { > + rk3066_hdmi_phy_write(hdmi, 0x158, 0x06); > + rk3066_hdmi_phy_write(hdmi, 0x15c, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x160, 0x60); > + rk3066_hdmi_phy_write(hdmi, 0x164, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x168, 0xCA); > + rk3066_hdmi_phy_write(hdmi, 0x16c, 0xA3); > + rk3066_hdmi_phy_write(hdmi, 0x170, 0x0e); > + rk3066_hdmi_phy_write(hdmi, 0x174, 0x20); > + rk3066_hdmi_phy_write(hdmi, 0x178, 0x00); > + } else { > + rk3066_hdmi_phy_write(hdmi, 0x158, 0x02); > + rk3066_hdmi_phy_write(hdmi, 0x15c, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x160, 0x60); > + rk3066_hdmi_phy_write(hdmi, 0x164, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x168, 0xC2); > + rk3066_hdmi_phy_write(hdmi, 0x16c, 0xA2); > + rk3066_hdmi_phy_write(hdmi, 0x170, 0x0e); > + rk3066_hdmi_phy_write(hdmi, 0x174, 0x20); > + rk3066_hdmi_phy_write(hdmi, 0x178, 0x00); > + } > +} > +static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder) > +{ > + struct rk3066_hdmi *hdmi =3D to_rk3066_hdmi(encoder); > + int mux, val; > + > + mux =3D drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder); > + if (mux) > + val =3D BIT(30) | BIT(14); > + else > + val =3D BIT(30); > + > + regmap_write(hdmi->grf, 0x150, val); Please define constants for both BIT(14) and the 0x150 GRF register, which is GRF_SOC_CON0, so in the header #define GRF_SOC_CON0 0x150 #define HDMI_VIDEO_SEL BIT(14) and then do if (mux) val =3D (HDMI_VIDEO_SEL << 16) | HDMI_VIDEO_SEL; else val =3D HDMI_VIDEO_SEL << 16; regmap_write(hdmi->grf, GRF_SOC_CON0, val); > + > + dev_dbg(hdmi->dev, "hdmi encoder enable select: vop%s\n", > + (mux) ? "1" : "0"); > + > + rk3066_hdmi_setup(hdmi, &hdmi->previous_mode); > +} > + > +static const struct drm_display_mode edid_cea_modes[] =3D { > + /* 4 - 1280x720@60Hz 16:9 */ > + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390, > + 1430, 1650, 0, 720, 725, 730, 750, 0, > + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), > + .vrefresh =3D 60, .picture_aspect_ratio =3D HDMI_PICTURE_ASPECT_16_9,= }, > +}; please drop that, see below > +static int rk3066_hdmi_connector_get_modes(struct drm_connector *connect= or) > +{ > + struct rk3066_hdmi *hdmi =3D to_rk3066_hdmi(connector); > + struct drm_display_mode *mode =3D NULL; > + struct edid *edid; > + int ret =3D 0; > + > + if (!hdmi->ddc) > + return 0; > + > + hdmi->hdmi_data.sink_is_hdmi =3D false; > + > + edid =3D drm_get_edid(connector, hdmi->ddc); > + if (edid) { > + hdmi->hdmi_data.sink_is_hdmi =3D drm_detect_hdmi_monitor(edid); > + > + dev_info(hdmi->dev, "monitor type : %s : %dx%d cm\n", > + (hdmi->hdmi_data.sink_is_hdmi ? "HDMI" : "DVI"), > + edid->width_cm, edid->height_cm); > + > + drm_connector_update_edid_property(connector, edid); > + ret =3D drm_add_edid_modes(connector, edid); > + kfree(edid); > + } > + > + if ((ret =3D=3D 0) || (hdmi->hdmi_data.sink_is_hdmi =3D=3D false)) { > + hdmi->hdmi_data.sink_is_hdmi =3D false; > + > + mode =3D drm_mode_duplicate(hdmi->drm_dev, &edid_cea_modes[0]); > + if (!mode) > + return ret; > + mode->type |=3D DRM_MODE_TYPE_PREFERRED; > + drm_mode_probed_add(connector, mode); > + ret++; > + > + dev_info(hdmi->dev, "no CEA mode found, use default\n"); This is a hack from the vendor-kernel. If EDID reading fails, it is some sort of issue with the driver, so we shouldn't assume any specific mode at all. See the somewhat similar inno_hdmi driver for comparison. > + } > + > + return ret; > +} > + > +static int > +rk3066_hdmi_register(struct drm_device *drm, struct rk3066_hdmi *hdmi) > +{ > + struct drm_encoder *encoder =3D &hdmi->encoder; > + struct device *dev =3D hdmi->dev; > + > + encoder->possible_crtcs =3D > + drm_of_find_possible_crtcs(drm, dev->of_node); > + > + /* > + * If we failed to find the VOP(s) which this encoder is > + * supposed to be connected to, it's because the VOP has > + * not been registered yet. Defer probing, and hope that > + * the required VOP is added later. > + */ > + if (encoder->possible_crtcs =3D=3D 0) > + return -EPROBE_DEFER; > + > + dev_info(hdmi->dev, "VOP found\n"); unnecessary output which will clutter up the kernel log, please drop. > + > + drm_encoder_helper_add(encoder, &rk3066_hdmi_encoder_helper_funcs); > + drm_encoder_init(drm, encoder, &rk3066_hdmi_encoder_funcs, > + DRM_MODE_ENCODER_TMDS, NULL); > + > + hdmi->connector.polled =3D DRM_CONNECTOR_POLL_HPD; > + > + drm_connector_helper_add(&hdmi->connector, > + &rk3066_hdmi_connector_helper_funcs); > + drm_connector_init(drm, &hdmi->connector, > + &rk3066_hdmi_connector_funcs, > + DRM_MODE_CONNECTOR_HDMIA); > + > + drm_connector_attach_encoder(&hdmi->connector, encoder); > + > + return 0; > +} > + > +static irqreturn_t rk3066_hdmi_i2c_irq(struct rk3066_hdmi *hdmi, u8 stat) > +{ > + struct rk3066_hdmi_i2c *i2c =3D hdmi->i2c; > + > + if (!(stat & HDMI_INTR_EDID_MASK)) > + return IRQ_NONE; > + > + i2c->stat =3D stat; > + > + complete(&i2c->compl); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t rk3066_hdmi_hardirq(int irq, void *dev_id) > +{ > + struct rk3066_hdmi *hdmi =3D dev_id; > + irqreturn_t ret =3D IRQ_NONE; > + u8 interrupt; > + > + if (rk3066_hdmi_get_power_mode(hdmi) =3D=3D HDMI_SYS_POWER_MODE_A) > + hdmi_writeb(hdmi, HDMI_SYS_CTRL, HDMI_SYS_POWER_MODE_B); > + > + interrupt =3D hdmi_readb(hdmi, HDMI_INTR_STATUS1); > + if (interrupt) > + hdmi_writeb(hdmi, HDMI_INTR_STATUS1, interrupt); > + > + if (hdmi->i2c) > + ret =3D rk3066_hdmi_i2c_irq(hdmi, interrupt); I think you don't really need that rk3066_hdmi_i2c_irq function above, this can just become if (interrupt & HDMI_INTR_EDID_MASK) { hdmi->i2c->stat =3D stat; complete(&hdmi->i2c->compl); } hdmi->i2c is set through rk3066_hdmi_i2c_adapter() which is always called and needs to be sucessful before registering the interrupt, so there is no need to check for hdmi->i2c here at all. > + > + if (interrupt & (HDMI_INTR_HOTPLUG | HDMI_INTR_MSENS)) > + ret =3D IRQ_WAKE_THREAD; > + > + return ret; > +} > + > +static struct i2c_adapter *rk3066_hdmi_i2c_adapter(struct rk3066_hdmi *h= dmi) > +{ > + struct i2c_adapter *adap; > + struct rk3066_hdmi_i2c *i2c; > + int ret; > + > + i2c =3D devm_kzalloc(hdmi->dev, sizeof(*i2c), GFP_KERNEL); > + if (!i2c) > + return ERR_PTR(-ENOMEM); > + > + mutex_init(&i2c->i2c_lock); > + init_completion(&i2c->compl); > + > + adap =3D &i2c->adap; > + adap->class =3D I2C_CLASS_DDC; > + adap->owner =3D THIS_MODULE; > + adap->dev.parent =3D hdmi->dev; > + adap->dev.of_node =3D hdmi->dev->of_node; > + adap->algo =3D &rk3066_hdmi_algorithm; > + strlcpy(adap->name, "RK3066 HDMI", sizeof(adap->name)); > + i2c_set_adapdata(adap, hdmi); > + > + ret =3D i2c_add_adapter(adap); > + if (ret) { > + dev_warn(hdmi->dev, "cannot add %s I2C adapter\n", adap->name); > + devm_kfree(hdmi->dev, i2c); > + return ERR_PTR(ret); > + } > + > + hdmi->i2c =3D i2c; > + > + dev_info(hdmi->dev, "registered %s I2C bus driver\n", adap->name); Please drop or make it a DRM_DEV_DEBUG. Also, please convert all the general dev_foobar calls to their DRM_* equivalents, so dev_warn becomes DRM_DEV_ERROR, dev_info DRM_DEV_INFO and so on. > + return adap; > +} > + > +static int rk3066_hdmi_bind(struct device *dev, struct device *master, > + void *data) > +{ > + struct platform_device *pdev =3D to_platform_device(dev); > + struct drm_device *drm =3D data; > + struct rk3066_hdmi *hdmi; > + struct resource *iores; > + int irq; > + int ret; > + > + hdmi =3D devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL); > + if (!hdmi) > + return -ENOMEM; > + > + hdmi->dev =3D dev; > + hdmi->drm_dev =3D drm; > + > + iores =3D platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!iores) > + return -ENXIO; > + > + hdmi->regs =3D devm_ioremap_resource(dev, iores); > + if (IS_ERR(hdmi->regs)) > + return PTR_ERR(hdmi->regs); > + > + irq =3D platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + hdmi->hclk =3D devm_clk_get(dev, "hclk"); > + if (IS_ERR(hdmi->hclk)) { > + dev_err(dev, "unable to get HDMI hclk clock\n"); > + return PTR_ERR(hdmi->hclk); > + } > + > + ret =3D clk_prepare_enable(hdmi->hclk); > + if (ret) { > + dev_err(dev, "cannot enable HDMI hclk clock: %d\n", ret); > + return ret; > + } > + > + hdmi->grf =3D syscon_regmap_lookup_by_phandle(dev->of_node, > + "rockchip,grf"); > + if (IS_ERR(hdmi->grf)) { > + dev_err(dev, "unable to get rockchip,grf\n"); > + ret =3D PTR_ERR(hdmi->grf); > + goto err_disable_hclk; > + } > + > + /* internal hclk =3D hdmi_hclk / 25 */ > + hdmi_writeb(hdmi, HDMI_INTERNAL_CLK_DIVIDER, 25); > + > + hdmi->ddc =3D rk3066_hdmi_i2c_adapter(hdmi); > + if (IS_ERR(hdmi->ddc)) { > + ret =3D PTR_ERR(hdmi->ddc); > + hdmi->ddc =3D NULL; > + goto err_disable_hclk; > + } > + > + rk3066_hdmi_set_power_mode(hdmi, HDMI_SYS_POWER_MODE_B); > + usleep_range(999, 1000); > + hdmi_writeb(hdmi, HDMI_INTR_MASK1, HDMI_INTR_HOTPLUG); > + hdmi_writeb(hdmi, HDMI_INTR_MASK2, 0); > + hdmi_writeb(hdmi, HDMI_INTR_MASK3, 0); > + hdmi_writeb(hdmi, HDMI_INTR_MASK4, 0); > + rk3066_hdmi_set_power_mode(hdmi, HDMI_SYS_POWER_MODE_A); > + > + ret =3D rk3066_hdmi_register(drm, hdmi); > + if (ret) > + goto err_disable_hclk; goto err_disable_i2c; So that the i2c-adapter also gets disabled on error. > + dev_set_drvdata(dev, hdmi); > + > + ret =3D devm_request_threaded_irq(dev, irq, rk3066_hdmi_hardirq, > + rk3066_hdmi_irq, IRQF_SHARED, > + dev_name(dev), hdmi); > + if (ret) { > + dev_err(dev, "failed to request hdmi irq: %d\n", ret); > + goto err_disable_hclk; goto err_disable_i2c; > + } > + > + return 0; > + err_disable_i2c: i2c_put_adapter(hdmi->ddc); > +err_disable_hclk: > + clk_disable_unprepare(hdmi->hclk); > + > + return ret; > +} > + > +static void rk3066_hdmi_unbind(struct device *dev, struct device *master, > + void *data) > +{ > + struct rk3066_hdmi *hdmi =3D dev_get_drvdata(dev); > + > + hdmi->connector.funcs->destroy(&hdmi->connector); > + hdmi->encoder.funcs->destroy(&hdmi->encoder); > + > + clk_disable_unprepare(hdmi->hclk); > + i2c_put_adapter(hdmi->ddc); you should probably invert the calling order here, first remove the i2c adapter and only then disable the clock. Thanks Heiko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 90519C43381 for ; Tue, 19 Mar 2019 11:44:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 559D520857 for ; Tue, 19 Mar 2019 11:44:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726712AbfCSLoS convert rfc822-to-8bit (ORCPT ); Tue, 19 Mar 2019 07:44:18 -0400 Received: from gloria.sntech.de ([185.11.138.130]:58294 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726002AbfCSLoR (ORCPT ); Tue, 19 Mar 2019 07:44:17 -0400 Received: from ip5f5a6320.dynamic.kabel-deutschland.de ([95.90.99.32] helo=diego.localnet) by gloria.sntech.de with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1h6DA4-0006B0-Ec; Tue, 19 Mar 2019 12:44:04 +0100 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Johan Jonker Cc: hjc@rock-chips.com, airlied@linux.ie, daniel@ffwll.ch, robh+dt@kernel.org, mark.rutland@arm.com, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 1/4] drm: rockchip: introduce rk3066 hdmi Date: Tue, 19 Mar 2019 12:44:03 +0100 Message-ID: <4125149.ebfS1HlHkS@diego> In-Reply-To: <20190306224113.24853-2-jbx6244@gmail.com> References: <20190306224113.24853-1-jbx6244@gmail.com> <20190306224113.24853-2-jbx6244@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Johan, Am Mittwoch, 6. März 2019, 23:41:10 CET schrieb Johan Jonker: > From: Zheng Yang > > The RK3066 HDMI TX serves as interface between a LCD Controller and > a HDMI bus. A HDMI TX consists of one HDMI transmitter controller and > one HDMI transmitter PHY. The interface has three (3) 8-bit data channels > which can be configured for a number of bus widths (8/10/12/16/20/24-bit) > and different video formats (RGB, YCbCr). > > Features: > HDMI version 1.4a, HDCP revision 1.4 and > DVI version 1.0 compliant transmitter. > Supports DTV resolutions from 480i to 1080i/p HD. > Master I2C interface for a DDC connection. > HDMI TX supports multiple power save modes. > The HDMI TX input can switch between LCDC0 and LCDC1. > (Sound support is not included in this patch) > > Signed-off-by: Zheng Yang > Signed-off-by: Johan Jonker Looks good in general, but there are some minor things to improve yet, please see below for specifics. [...] > +static void rk3066_hdmi_config_phy(struct rk3066_hdmi *hdmi) > +{ > + /* TMDS uses the same frequency as dclk. */ > + hdmi_writeb(hdmi, HDMI_DEEP_COLOR_MODE, 0x22); These magic values below are no fault of yours, but in any case this could use a comment that the semi-public documentation does not describe hdmi-registers at all, so we're stuck with these magic-values for now. > + if (hdmi->tmdsclk > 100000000) { > + rk3066_hdmi_phy_write(hdmi, 0x158, 0x0E); > + rk3066_hdmi_phy_write(hdmi, 0x15c, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x160, 0x60); > + rk3066_hdmi_phy_write(hdmi, 0x164, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x168, 0xDA); > + rk3066_hdmi_phy_write(hdmi, 0x16c, 0xA1); > + rk3066_hdmi_phy_write(hdmi, 0x170, 0x0e); > + rk3066_hdmi_phy_write(hdmi, 0x174, 0x22); > + rk3066_hdmi_phy_write(hdmi, 0x178, 0x00); > + } else if (hdmi->tmdsclk > 50000000) { > + rk3066_hdmi_phy_write(hdmi, 0x158, 0x06); > + rk3066_hdmi_phy_write(hdmi, 0x15c, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x160, 0x60); > + rk3066_hdmi_phy_write(hdmi, 0x164, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x168, 0xCA); > + rk3066_hdmi_phy_write(hdmi, 0x16c, 0xA3); > + rk3066_hdmi_phy_write(hdmi, 0x170, 0x0e); > + rk3066_hdmi_phy_write(hdmi, 0x174, 0x20); > + rk3066_hdmi_phy_write(hdmi, 0x178, 0x00); > + } else { > + rk3066_hdmi_phy_write(hdmi, 0x158, 0x02); > + rk3066_hdmi_phy_write(hdmi, 0x15c, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x160, 0x60); > + rk3066_hdmi_phy_write(hdmi, 0x164, 0x00); > + rk3066_hdmi_phy_write(hdmi, 0x168, 0xC2); > + rk3066_hdmi_phy_write(hdmi, 0x16c, 0xA2); > + rk3066_hdmi_phy_write(hdmi, 0x170, 0x0e); > + rk3066_hdmi_phy_write(hdmi, 0x174, 0x20); > + rk3066_hdmi_phy_write(hdmi, 0x178, 0x00); > + } > +} > +static void rk3066_hdmi_encoder_enable(struct drm_encoder *encoder) > +{ > + struct rk3066_hdmi *hdmi = to_rk3066_hdmi(encoder); > + int mux, val; > + > + mux = drm_of_encoder_active_endpoint_id(hdmi->dev->of_node, encoder); > + if (mux) > + val = BIT(30) | BIT(14); > + else > + val = BIT(30); > + > + regmap_write(hdmi->grf, 0x150, val); Please define constants for both BIT(14) and the 0x150 GRF register, which is GRF_SOC_CON0, so in the header #define GRF_SOC_CON0 0x150 #define HDMI_VIDEO_SEL BIT(14) and then do if (mux) val = (HDMI_VIDEO_SEL << 16) | HDMI_VIDEO_SEL; else val = HDMI_VIDEO_SEL << 16; regmap_write(hdmi->grf, GRF_SOC_CON0, val); > + > + dev_dbg(hdmi->dev, "hdmi encoder enable select: vop%s\n", > + (mux) ? "1" : "0"); > + > + rk3066_hdmi_setup(hdmi, &hdmi->previous_mode); > +} > + > +static const struct drm_display_mode edid_cea_modes[] = { > + /* 4 - 1280x720@60Hz 16:9 */ > + { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390, > + 1430, 1650, 0, 720, 725, 730, 750, 0, > + DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC), > + .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, > +}; please drop that, see below > +static int rk3066_hdmi_connector_get_modes(struct drm_connector *connector) > +{ > + struct rk3066_hdmi *hdmi = to_rk3066_hdmi(connector); > + struct drm_display_mode *mode = NULL; > + struct edid *edid; > + int ret = 0; > + > + if (!hdmi->ddc) > + return 0; > + > + hdmi->hdmi_data.sink_is_hdmi = false; > + > + edid = drm_get_edid(connector, hdmi->ddc); > + if (edid) { > + hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid); > + > + dev_info(hdmi->dev, "monitor type : %s : %dx%d cm\n", > + (hdmi->hdmi_data.sink_is_hdmi ? "HDMI" : "DVI"), > + edid->width_cm, edid->height_cm); > + > + drm_connector_update_edid_property(connector, edid); > + ret = drm_add_edid_modes(connector, edid); > + kfree(edid); > + } > + > + if ((ret == 0) || (hdmi->hdmi_data.sink_is_hdmi == false)) { > + hdmi->hdmi_data.sink_is_hdmi = false; > + > + mode = drm_mode_duplicate(hdmi->drm_dev, &edid_cea_modes[0]); > + if (!mode) > + return ret; > + mode->type |= DRM_MODE_TYPE_PREFERRED; > + drm_mode_probed_add(connector, mode); > + ret++; > + > + dev_info(hdmi->dev, "no CEA mode found, use default\n"); This is a hack from the vendor-kernel. If EDID reading fails, it is some sort of issue with the driver, so we shouldn't assume any specific mode at all. See the somewhat similar inno_hdmi driver for comparison. > + } > + > + return ret; > +} > + > +static int > +rk3066_hdmi_register(struct drm_device *drm, struct rk3066_hdmi *hdmi) > +{ > + struct drm_encoder *encoder = &hdmi->encoder; > + struct device *dev = hdmi->dev; > + > + encoder->possible_crtcs = > + drm_of_find_possible_crtcs(drm, dev->of_node); > + > + /* > + * If we failed to find the VOP(s) which this encoder is > + * supposed to be connected to, it's because the VOP has > + * not been registered yet. Defer probing, and hope that > + * the required VOP is added later. > + */ > + if (encoder->possible_crtcs == 0) > + return -EPROBE_DEFER; > + > + dev_info(hdmi->dev, "VOP found\n"); unnecessary output which will clutter up the kernel log, please drop. > + > + drm_encoder_helper_add(encoder, &rk3066_hdmi_encoder_helper_funcs); > + drm_encoder_init(drm, encoder, &rk3066_hdmi_encoder_funcs, > + DRM_MODE_ENCODER_TMDS, NULL); > + > + hdmi->connector.polled = DRM_CONNECTOR_POLL_HPD; > + > + drm_connector_helper_add(&hdmi->connector, > + &rk3066_hdmi_connector_helper_funcs); > + drm_connector_init(drm, &hdmi->connector, > + &rk3066_hdmi_connector_funcs, > + DRM_MODE_CONNECTOR_HDMIA); > + > + drm_connector_attach_encoder(&hdmi->connector, encoder); > + > + return 0; > +} > + > +static irqreturn_t rk3066_hdmi_i2c_irq(struct rk3066_hdmi *hdmi, u8 stat) > +{ > + struct rk3066_hdmi_i2c *i2c = hdmi->i2c; > + > + if (!(stat & HDMI_INTR_EDID_MASK)) > + return IRQ_NONE; > + > + i2c->stat = stat; > + > + complete(&i2c->compl); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t rk3066_hdmi_hardirq(int irq, void *dev_id) > +{ > + struct rk3066_hdmi *hdmi = dev_id; > + irqreturn_t ret = IRQ_NONE; > + u8 interrupt; > + > + if (rk3066_hdmi_get_power_mode(hdmi) == HDMI_SYS_POWER_MODE_A) > + hdmi_writeb(hdmi, HDMI_SYS_CTRL, HDMI_SYS_POWER_MODE_B); > + > + interrupt = hdmi_readb(hdmi, HDMI_INTR_STATUS1); > + if (interrupt) > + hdmi_writeb(hdmi, HDMI_INTR_STATUS1, interrupt); > + > + if (hdmi->i2c) > + ret = rk3066_hdmi_i2c_irq(hdmi, interrupt); I think you don't really need that rk3066_hdmi_i2c_irq function above, this can just become if (interrupt & HDMI_INTR_EDID_MASK) { hdmi->i2c->stat = stat; complete(&hdmi->i2c->compl); } hdmi->i2c is set through rk3066_hdmi_i2c_adapter() which is always called and needs to be sucessful before registering the interrupt, so there is no need to check for hdmi->i2c here at all. > + > + if (interrupt & (HDMI_INTR_HOTPLUG | HDMI_INTR_MSENS)) > + ret = IRQ_WAKE_THREAD; > + > + return ret; > +} > + > +static struct i2c_adapter *rk3066_hdmi_i2c_adapter(struct rk3066_hdmi *hdmi) > +{ > + struct i2c_adapter *adap; > + struct rk3066_hdmi_i2c *i2c; > + int ret; > + > + i2c = devm_kzalloc(hdmi->dev, sizeof(*i2c), GFP_KERNEL); > + if (!i2c) > + return ERR_PTR(-ENOMEM); > + > + mutex_init(&i2c->i2c_lock); > + init_completion(&i2c->compl); > + > + adap = &i2c->adap; > + adap->class = I2C_CLASS_DDC; > + adap->owner = THIS_MODULE; > + adap->dev.parent = hdmi->dev; > + adap->dev.of_node = hdmi->dev->of_node; > + adap->algo = &rk3066_hdmi_algorithm; > + strlcpy(adap->name, "RK3066 HDMI", sizeof(adap->name)); > + i2c_set_adapdata(adap, hdmi); > + > + ret = i2c_add_adapter(adap); > + if (ret) { > + dev_warn(hdmi->dev, "cannot add %s I2C adapter\n", adap->name); > + devm_kfree(hdmi->dev, i2c); > + return ERR_PTR(ret); > + } > + > + hdmi->i2c = i2c; > + > + dev_info(hdmi->dev, "registered %s I2C bus driver\n", adap->name); Please drop or make it a DRM_DEV_DEBUG. Also, please convert all the general dev_foobar calls to their DRM_* equivalents, so dev_warn becomes DRM_DEV_ERROR, dev_info DRM_DEV_INFO and so on. > + return adap; > +} > + > +static int rk3066_hdmi_bind(struct device *dev, struct device *master, > + void *data) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct drm_device *drm = data; > + struct rk3066_hdmi *hdmi; > + struct resource *iores; > + int irq; > + int ret; > + > + hdmi = devm_kzalloc(dev, sizeof(*hdmi), GFP_KERNEL); > + if (!hdmi) > + return -ENOMEM; > + > + hdmi->dev = dev; > + hdmi->drm_dev = drm; > + > + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!iores) > + return -ENXIO; > + > + hdmi->regs = devm_ioremap_resource(dev, iores); > + if (IS_ERR(hdmi->regs)) > + return PTR_ERR(hdmi->regs); > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > + > + hdmi->hclk = devm_clk_get(dev, "hclk"); > + if (IS_ERR(hdmi->hclk)) { > + dev_err(dev, "unable to get HDMI hclk clock\n"); > + return PTR_ERR(hdmi->hclk); > + } > + > + ret = clk_prepare_enable(hdmi->hclk); > + if (ret) { > + dev_err(dev, "cannot enable HDMI hclk clock: %d\n", ret); > + return ret; > + } > + > + hdmi->grf = syscon_regmap_lookup_by_phandle(dev->of_node, > + "rockchip,grf"); > + if (IS_ERR(hdmi->grf)) { > + dev_err(dev, "unable to get rockchip,grf\n"); > + ret = PTR_ERR(hdmi->grf); > + goto err_disable_hclk; > + } > + > + /* internal hclk = hdmi_hclk / 25 */ > + hdmi_writeb(hdmi, HDMI_INTERNAL_CLK_DIVIDER, 25); > + > + hdmi->ddc = rk3066_hdmi_i2c_adapter(hdmi); > + if (IS_ERR(hdmi->ddc)) { > + ret = PTR_ERR(hdmi->ddc); > + hdmi->ddc = NULL; > + goto err_disable_hclk; > + } > + > + rk3066_hdmi_set_power_mode(hdmi, HDMI_SYS_POWER_MODE_B); > + usleep_range(999, 1000); > + hdmi_writeb(hdmi, HDMI_INTR_MASK1, HDMI_INTR_HOTPLUG); > + hdmi_writeb(hdmi, HDMI_INTR_MASK2, 0); > + hdmi_writeb(hdmi, HDMI_INTR_MASK3, 0); > + hdmi_writeb(hdmi, HDMI_INTR_MASK4, 0); > + rk3066_hdmi_set_power_mode(hdmi, HDMI_SYS_POWER_MODE_A); > + > + ret = rk3066_hdmi_register(drm, hdmi); > + if (ret) > + goto err_disable_hclk; goto err_disable_i2c; So that the i2c-adapter also gets disabled on error. > + dev_set_drvdata(dev, hdmi); > + > + ret = devm_request_threaded_irq(dev, irq, rk3066_hdmi_hardirq, > + rk3066_hdmi_irq, IRQF_SHARED, > + dev_name(dev), hdmi); > + if (ret) { > + dev_err(dev, "failed to request hdmi irq: %d\n", ret); > + goto err_disable_hclk; goto err_disable_i2c; > + } > + > + return 0; > + err_disable_i2c: i2c_put_adapter(hdmi->ddc); > +err_disable_hclk: > + clk_disable_unprepare(hdmi->hclk); > + > + return ret; > +} > + > +static void rk3066_hdmi_unbind(struct device *dev, struct device *master, > + void *data) > +{ > + struct rk3066_hdmi *hdmi = dev_get_drvdata(dev); > + > + hdmi->connector.funcs->destroy(&hdmi->connector); > + hdmi->encoder.funcs->destroy(&hdmi->encoder); > + > + clk_disable_unprepare(hdmi->hclk); > + i2c_put_adapter(hdmi->ddc); you should probably invert the calling order here, first remove the i2c adapter and only then disable the clock. Thanks Heiko