From mboxrd@z Thu Jan 1 00:00:00 1970 From: moinejf@free.fr (Jean-Francois Moine) Date: Wed, 17 Jul 2013 20:30:14 +0200 Subject: DT binding review for Armada display subsystem In-Reply-To: References: <20130712164428.AC3D2FAAE9@dev.laptop.org> <20130715210900.GT14452@pengutronix.de> Message-ID: <20130717203014.339cd37c@armhf> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, 17 Jul 2013 11:50:11 -0600 Daniel Drake wrote: > I am now facing a problem with i2c/TDA998x which Russell already noted: > > http://lists.freedesktop.org/archives/dri-devel/2013-June/039632.html > What *can't* be done without a rewrite of the DRM slave encoder backend > is to have the TDA998x I2C device provided by DT. There are mumblings > about redoing the slave encoder API, hopefully this will be fixed there. > > This is because the existing DRM/encoder system works something like this: > 1. i2c driver (e.g. tda998x_drv) registers as an i2c_driver via > drm_i2c_encoder_register() > 2. The drm driver (i.e. armada) later has to know that it is expecting > a tda998x instance. It calls drm_i2c_encoder_init() to set this up. > > drm_i2c_encoder init requires: > 1. The i2c_adapter in question. In a DT world, we could get this from > finding the parent node of the tda998x node (this is the i2c bus > master), and then using i2c_for_each_dev to find the first i2c adapter > instance that has the same of_node. > 2. i2c_board_info - basically the address of the device on the i2c > bus. This is basically the way of instantiating i2c devices from > platform data. Not the DT way :( > > In a DT world the i2c driver would be instantiated from a node in the > DT, which already includes the info that would come in i2c_board_info. > Then later it would have to be linked to a DRM slave/encoder, perhaps > when the DRM device finds the of_node corresponding to it. > > So I think the next step is fixing up DRM, as Russell hinted. Unless > someone sees another acceptable option that doesn't break our DT > design. Hi Daniel, I don't see the problem with the TDA998x. Indeed, it needs some enhancement to handle the DT, but also, the function drm_i2c_encoder_init() is not usable in the DT case (this function loads the module, while the module must be loaded by the DT for it gets all parameters - i2c address and irq). Here is the simple solution I use: - the tda998x is declared in the DT, so the module is loaded at system startup time. - its probe function is void. - when the connector of the drm driver scans the DT, it finds the phandle to the tda998x. - if / after the tda998x is loaded, the connector calls the function encoder_init() of the tda998x (and also increment the reference counter of the tda998x module to avoid this last one to be unloaded). - the function encoder_init() of the tda998x scans the DT and it has all elements to run. This working is compatible with the ticlcd driver: calling drm_i2c_encoder_init() just prevents the tda998x to use the irq (the i2c address is hard coded, the display connect/disconnect event is detected by polling and reading the EDID is synchronous). -- Ken ar c'henta? | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean-Francois Moine Subject: Re: DT binding review for Armada display subsystem Date: Wed, 17 Jul 2013 20:30:14 +0200 Message-ID: <20130717203014.339cd37c@armhf> References: <20130712164428.AC3D2FAAE9@dev.laptop.org> <20130715210900.GT14452@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Daniel Drake Cc: Sascha Hauer , dri-devel@lists.freedesktop.org, Russell King , "devicetree-discuss@lists.ozlabs.org" , linux-arm-kernel , Sebastian Hesselbarth List-Id: devicetree@vger.kernel.org T24gV2VkLCAxNyBKdWwgMjAxMyAxMTo1MDoxMSAtMDYwMApEYW5pZWwgRHJha2UgPGRzZEBsYXB0 b3Aub3JnPiB3cm90ZToKCj4gSSBhbSBub3cgZmFjaW5nIGEgcHJvYmxlbSB3aXRoIGkyYy9UREE5 OTh4IHdoaWNoIFJ1c3NlbGwgYWxyZWFkeSBub3RlZDoKPiAKPiBodHRwOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL2FyY2hpdmVzL2RyaS1kZXZlbC8yMDEzLUp1bmUvMDM5NjMyLmh0bWwKPiAgICBX aGF0ICpjYW4ndCogYmUgZG9uZSB3aXRob3V0IGEgcmV3cml0ZSBvZiB0aGUgRFJNIHNsYXZlIGVu Y29kZXIgYmFja2VuZAo+ICAgIGlzIHRvIGhhdmUgdGhlIFREQTk5OHggSTJDIGRldmljZSBwcm92 aWRlZCBieSBEVC4gIFRoZXJlIGFyZSBtdW1ibGluZ3MKPiAgICBhYm91dCByZWRvaW5nIHRoZSBz bGF2ZSBlbmNvZGVyIEFQSSwgaG9wZWZ1bGx5IHRoaXMgd2lsbCBiZSBmaXhlZCB0aGVyZS4KPiAK PiBUaGlzIGlzIGJlY2F1c2UgdGhlIGV4aXN0aW5nIERSTS9lbmNvZGVyIHN5c3RlbSB3b3JrcyBz b21ldGhpbmcgbGlrZSB0aGlzOgo+IDEuIGkyYyBkcml2ZXIgKGUuZy4gdGRhOTk4eF9kcnYpIHJl Z2lzdGVycyBhcyBhbiBpMmNfZHJpdmVyIHZpYQo+IGRybV9pMmNfZW5jb2Rlcl9yZWdpc3Rlcigp Cj4gMi4gVGhlIGRybSBkcml2ZXIgKGkuZS4gYXJtYWRhKSBsYXRlciBoYXMgdG8ga25vdyB0aGF0 IGl0IGlzIGV4cGVjdGluZwo+IGEgdGRhOTk4eCBpbnN0YW5jZS4gSXQgY2FsbHMgZHJtX2kyY19l bmNvZGVyX2luaXQoKSB0byBzZXQgdGhpcyB1cC4KPiAKPiBkcm1faTJjX2VuY29kZXIgaW5pdCBy ZXF1aXJlczoKPiAgMS4gVGhlIGkyY19hZGFwdGVyIGluIHF1ZXN0aW9uLiBJbiBhIERUIHdvcmxk LCB3ZSBjb3VsZCBnZXQgdGhpcyBmcm9tCj4gZmluZGluZyB0aGUgcGFyZW50IG5vZGUgb2YgdGhl IHRkYTk5OHggbm9kZSAodGhpcyBpcyB0aGUgaTJjIGJ1cwo+IG1hc3RlciksIGFuZCB0aGVuIHVz aW5nIGkyY19mb3JfZWFjaF9kZXYgdG8gZmluZCB0aGUgZmlyc3QgaTJjIGFkYXB0ZXIKPiBpbnN0 YW5jZSB0aGF0IGhhcyB0aGUgc2FtZSBvZl9ub2RlLgo+ICAyLiBpMmNfYm9hcmRfaW5mbyAtIGJh c2ljYWxseSB0aGUgYWRkcmVzcyBvZiB0aGUgZGV2aWNlIG9uIHRoZSBpMmMKPiBidXMuIFRoaXMg aXMgYmFzaWNhbGx5IHRoZSB3YXkgb2YgaW5zdGFudGlhdGluZyBpMmMgZGV2aWNlcyBmcm9tCj4g cGxhdGZvcm0gZGF0YS4gTm90IHRoZSBEVCB3YXkgOigKPiAKPiBJbiBhIERUIHdvcmxkIHRoZSBp MmMgZHJpdmVyIHdvdWxkIGJlIGluc3RhbnRpYXRlZCBmcm9tIGEgbm9kZSBpbiB0aGUKPiBEVCwg d2hpY2ggYWxyZWFkeSBpbmNsdWRlcyB0aGUgaW5mbyB0aGF0IHdvdWxkIGNvbWUgaW4gaTJjX2Jv YXJkX2luZm8uCj4gVGhlbiBsYXRlciBpdCB3b3VsZCBoYXZlIHRvIGJlIGxpbmtlZCB0byBhIERS TSBzbGF2ZS9lbmNvZGVyLCBwZXJoYXBzCj4gd2hlbiB0aGUgRFJNIGRldmljZSBmaW5kcyB0aGUg b2Zfbm9kZSBjb3JyZXNwb25kaW5nIHRvIGl0Lgo+IAo+IFNvIEkgdGhpbmsgdGhlIG5leHQgc3Rl cCBpcyBmaXhpbmcgdXAgRFJNLCBhcyBSdXNzZWxsIGhpbnRlZC4gVW5sZXNzCj4gc29tZW9uZSBz ZWVzIGFub3RoZXIgYWNjZXB0YWJsZSBvcHRpb24gdGhhdCBkb2Vzbid0IGJyZWFrIG91ciBEVAo+ IGRlc2lnbi4KCkhpIERhbmllbCwKCkkgZG9uJ3Qgc2VlIHRoZSBwcm9ibGVtIHdpdGggdGhlIFRE QTk5OHguCgpJbmRlZWQsIGl0IG5lZWRzIHNvbWUgZW5oYW5jZW1lbnQgdG8gaGFuZGxlIHRoZSBE VCwgYnV0IGFsc28sIHRoZQpmdW5jdGlvbiBkcm1faTJjX2VuY29kZXJfaW5pdCgpIGlzIG5vdCB1 c2FibGUgaW4gdGhlIERUIGNhc2UgKHRoaXMKZnVuY3Rpb24gbG9hZHMgdGhlIG1vZHVsZSwgd2hp bGUgdGhlIG1vZHVsZSBtdXN0IGJlIGxvYWRlZCBieSB0aGUgRFQKZm9yIGl0IGdldHMgYWxsIHBh cmFtZXRlcnMgLSBpMmMgYWRkcmVzcyBhbmQgaXJxKS4KCkhlcmUgaXMgdGhlIHNpbXBsZSBzb2x1 dGlvbiBJIHVzZToKCi0gdGhlIHRkYTk5OHggaXMgZGVjbGFyZWQgaW4gdGhlIERULCBzbyB0aGUg bW9kdWxlIGlzIGxvYWRlZCBhdCBzeXN0ZW0KICBzdGFydHVwIHRpbWUuCgotIGl0cyBwcm9iZSBm dW5jdGlvbiBpcyB2b2lkLgoKLSB3aGVuIHRoZSBjb25uZWN0b3Igb2YgdGhlIGRybSBkcml2ZXIg c2NhbnMgdGhlIERULCBpdCBmaW5kcyB0aGUKICBwaGFuZGxlIHRvIHRoZSB0ZGE5OTh4LgoKLSBp ZiAvIGFmdGVyIHRoZSB0ZGE5OTh4IGlzIGxvYWRlZCwgdGhlIGNvbm5lY3RvciBjYWxscyB0aGUg ZnVuY3Rpb24KICBlbmNvZGVyX2luaXQoKSBvZiB0aGUgdGRhOTk4eCAoYW5kIGFsc28gaW5jcmVt ZW50IHRoZSByZWZlcmVuY2UKICBjb3VudGVyIG9mIHRoZSB0ZGE5OTh4IG1vZHVsZSB0byBhdm9p ZCB0aGlzIGxhc3Qgb25lIHRvIGJlIHVubG9hZGVkKS4KCi0gdGhlIGZ1bmN0aW9uIGVuY29kZXJf aW5pdCgpIG9mIHRoZSB0ZGE5OTh4IHNjYW5zIHRoZSBEVCBhbmQgaXQgaGFzIGFsbAogIGVsZW1l bnRzIHRvIHJ1bi4KClRoaXMgd29ya2luZyBpcyBjb21wYXRpYmxlIHdpdGggdGhlIHRpY2xjZCBk cml2ZXI6IGNhbGxpbmcKZHJtX2kyY19lbmNvZGVyX2luaXQoKSBqdXN0IHByZXZlbnRzIHRoZSB0 ZGE5OTh4IHRvIHVzZSB0aGUgaXJxICh0aGUKaTJjIGFkZHJlc3MgaXMgaGFyZCBjb2RlZCwgdGhl IGRpc3BsYXkgY29ubmVjdC9kaXNjb25uZWN0IGV2ZW50IGlzCmRldGVjdGVkIGJ5IHBvbGxpbmcg YW5kIHJlYWRpbmcgdGhlIEVESUQgaXMgc3luY2hyb25vdXMpLgoKLS0gCktlbiBhciBjJ2hlbnRh w7EJfAkgICAgICAqKiBCcmVpemggaGEgTGludXggYXRhdiEgKioKSmVmCQl8CQlodHRwOi8vbW9p bmVqZi5mcmVlLmZyLwoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX18KbGludXgtYXJtLWtlcm5lbCBtYWlsaW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0 cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGlu Zm8vbGludXgtYXJtLWtlcm5lbAo=