From mboxrd@z Thu Jan 1 00:00:00 1970 From: moinejf@free.fr (Jean-Francois Moine) Date: Thu, 27 Mar 2014 12:34:49 +0100 Subject: [PATCH RFC v2 2/6] drm/i2c: tda998x: Move tda998x to a couple encoder/connector In-Reply-To: <6885089.l87kb3TNcV@avalon> References: <1458827.cQ6aDWdh1W@avalon> <20140325165548.0065b639@armhf> <6885089.l87kb3TNcV@avalon> Message-ID: <20140327123449.65584e93@armhf> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Laurent, On Wed, 26 Mar 2014 18:33:09 +0100 Laurent Pinchart wrote: > That could work in your case, but I don't really like that. > > We need to describe the hardware topology, that might be the only point we all > agree on. There are various hardware topologies we need to support with > different levels of complexity, and several ways to describe them, also with a > wide range of complexities and features. > > The more complex the hardware topology, the more complex its description needs > to be, and the more complex the code that will parse the description and > handle the hardware will be. I don't think there's any doubt here. But also, the simpler is the topology and the simpler should be its description. In your approach, the connections between endpoints are described in the components themselves, which makes hard for the DT maintainers to have a global understanding of the video subsystem. Having the topology described in the master device is clearer, even if it is complex. > A given device can be integrated in a wide variety of hardware with different > complexities. A driver can't thus always assume that the whole composite > display device will match a given class of topologies. This is especially true > for encoders and connectors, they can be hooked up directly at the output of a > very simple display controller, or can be part of a very complex graph. > Encoder and connector drivers can't assume much about how they will be > integrated. I want to avoid a situation where using an HDMI encoder already > supported in mainline with a different SoC will result in having to rewrite > the HDMI encoder driver. The tda998x chips are simple enough for being used in many boards: one video input and one serial digital output. I don't see in which circumstance the driver should be rewritten. > The story is a bit different for display controllers. While the hardware > topology outside (and sometimes inside as well) of the SoC can vary, a display > controller often approximately implies a given level of complexity. A cheap > SoC with a simple display controller will likely not be used to drive a 4k > display requiring 4 encoders running in parallel for instance. While I also > want to avoid having to make significant changes to a display controller > driver when using the SoC on a different board, I believe the requirement can > be slightly relaxed here, and the display controller driver(s) can assume more > about the hardware topology than the encoder drivers. I don't think that the display controllers or the encoders have to know about the topology. They have well-knwon specific functions and the way they are interconnected should not impact these functions. If that would be the case, they should offer a particular configuration interface to the master driver. > I've asked myself whether we needed a single, one-size-fits-them-all DT > representation of the hardware topology. The view of the world from an > external encoder point of view must not depend on the SoC it is hooked up to > (this would prevent using a single encoder driver with multiple SoCs), which > calls for at least some form of standardization. The topology representation > on the display controller side may vary from display controller to display > controller, but I believe this would just result in code duplication and > having to solve the same problem in multiple drivers. For those reasons I > believe that the OF graph proposal to represent the display hardware topology > would be a good choice. The bindings are flexible enough to represent both > simple and complex hardware. Your OF graph proposal is rather complex, and it does not even indicate which way the data flows. > Now, I don't want to force all display device drivers to implement complex > code when they only need to support simple hardware and simple hardware > topologies. Not only would that be rightfully rejected, I would be among the > ones nack'ing that approach. My opinion is that this calls for the creation of > helpers to handle common cases. Several (possibly many) display systems only > need (or want) to support linear pipelines at their output(s), made of a > single encoder and a single connector. There's no point in duplicating DT > parsing or encoder/connector instantiation code in several drivers in that > case where helpers could be reused. Several sets of helpers could support > different kinds of topologies, with the driver author selecting a set of > helpers depending on the expected hardware topology complexity. I don't like this 'helper' notion. See below. > We also need to decide on who (as in which driver) will be responsible for > binding all devices together. As DRM exposes a single device to userspace, > there needs to be a single driver that will perform front line handling of the > userspace API and delegate calls to the other drivers involved. I believe it > would be logical for that driver to also be in charge of bindings the > components together, as it will be the driver that delegate calls to the > components. For a similar reason I also believe that that driver should also > be the one in control of creating DRM encoders and DRM connectors. The > component drivers having only a narrow view of the device they handle, they > can't perform that task in a generic way and would thus get tied to specific > master drivers because of the assumptions they would make. I don't see why the encoders and connectors should be created by some external entity. They are declared in the DT or created by the board init, so, they exist at system startup time. > Whether the master driver is the CRTC driver or a separate driver that binds > standalone CRTCs, connectors and encoders doesn't in my opinion change my > above conclusions. Some SoCs could use a simple-video-card driver like the one > you've proposed, and others could implement a display controller driver that > would perform the same tasks for more complex pipelines in addition to > controlling the display controller's CRTC(s). The simple-video-card driver > would perform that same tasks as the helpers I've previously talked about, so > the two solutions are pretty similar, and I don't see much added value in the > general case in having a simple-video-card driver compared to using helpers in > the display controller driver. In fact, I wonder why there is not only one DRM driver. The global logic is always the same, and, actually, it is duplicated in each specific driver. I had this approach in the gspca driver: one main driver and many specific subdrivers. Once, I even had the idea to convert your UVC driver into a gspca subdriver, but, at this time, I had too much work with the other webcams. Nevertheless, it would have been easy: all the required interfaces were (are still) present in the main gspca driver. In the same order of idea, there is only one ALSA SoC driver, and you cannot say that the audio topology is less complex than the video one. When thinking about the unique DRM driver, there would no helper anymore: the driver would offer to each component the functions they need for working correctly. > The point that matters to me is that encoders DT bindings and drivers must be > identical regardless of whether the master driver is the display controller > driver or a driver for a logical composite device. For all those reasons we > should use the OF graph DT bindings for the simple-video-card driver should we > decide to implement it, and we should create DRM encoders and connectors in > the master driver, not in the encoder drivers. Sorry, but I think that a DRM encoder is the hardware encoder. What else could it be? At initialization time, the master driver has only to manage the relation between the video components. It has not to create entities which already exist. So my preferred scheme is: - at starting time, each video component initializes its software and hardware, and then, plugs itself into the DRM driver. - from the central video topology, the DRM device waits for all components to be plugged and then it links the components together, creating the user's view of the system. If you create the encoders and connectors in the DRM master, the hardware encoder and connector devices are just like zombies. They must put something in the system to say they are here and they must also have a callback function for them to know to which video subsystem they belong. On the contrary, if the DRM encoders and connectors are created by the hardware devices, they are immediately alive and operational. -- 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: [PATCH RFC v2 2/6] drm/i2c: tda998x: Move tda998x to a couple encoder/connector Date: Thu, 27 Mar 2014 12:34:49 +0100 Message-ID: <20140327123449.65584e93@armhf> References: <1458827.cQ6aDWdh1W@avalon> <20140325165548.0065b639@armhf> <6885089.l87kb3TNcV@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from smtp2-g21.free.fr (smtp2-g21.free.fr [212.27.42.2]) by gabe.freedesktop.org (Postfix) with ESMTP id D5B766E8BF for ; Thu, 27 Mar 2014 04:34:15 -0700 (PDT) In-Reply-To: <6885089.l87kb3TNcV@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart Cc: Russell King , linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgTGF1cmVudCwKCk9uIFdlZCwgMjYgTWFyIDIwMTQgMTg6MzM6MDkgKzAxMDAKTGF1cmVudCBQ aW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9hcmQuY29tPiB3cm90ZToKCj4gVGhh dCBjb3VsZCB3b3JrIGluIHlvdXIgY2FzZSwgYnV0IEkgZG9uJ3QgcmVhbGx5IGxpa2UgdGhhdC4K PiAKPiBXZSBuZWVkIHRvIGRlc2NyaWJlIHRoZSBoYXJkd2FyZSB0b3BvbG9neSwgdGhhdCBtaWdo dCBiZSB0aGUgb25seSBwb2ludCB3ZSBhbGwgCj4gYWdyZWUgb24uIFRoZXJlIGFyZSB2YXJpb3Vz IGhhcmR3YXJlIHRvcG9sb2dpZXMgd2UgbmVlZCB0byBzdXBwb3J0IHdpdGggCj4gZGlmZmVyZW50 IGxldmVscyBvZiBjb21wbGV4aXR5LCBhbmQgc2V2ZXJhbCB3YXlzIHRvIGRlc2NyaWJlIHRoZW0s IGFsc28gd2l0aCBhIAo+IHdpZGUgcmFuZ2Ugb2YgY29tcGxleGl0aWVzIGFuZCBmZWF0dXJlcy4K PiAKPiBUaGUgbW9yZSBjb21wbGV4IHRoZSBoYXJkd2FyZSB0b3BvbG9neSwgdGhlIG1vcmUgY29t cGxleCBpdHMgZGVzY3JpcHRpb24gbmVlZHMgCj4gdG8gYmUsIGFuZCB0aGUgbW9yZSBjb21wbGV4 IHRoZSBjb2RlIHRoYXQgd2lsbCBwYXJzZSB0aGUgZGVzY3JpcHRpb24gYW5kIAo+IGhhbmRsZSB0 aGUgaGFyZHdhcmUgd2lsbCBiZS4gSSBkb24ndCB0aGluayB0aGVyZSdzIGFueSBkb3VidCBoZXJl LgoKQnV0IGFsc28sIHRoZSBzaW1wbGVyIGlzIHRoZSB0b3BvbG9neSBhbmQgdGhlIHNpbXBsZXIg c2hvdWxkIGJlIGl0cwpkZXNjcmlwdGlvbi4KCkluIHlvdXIgYXBwcm9hY2gsIHRoZSBjb25uZWN0 aW9ucyBiZXR3ZWVuIGVuZHBvaW50cyBhcmUgZGVzY3JpYmVkIGluCnRoZSBjb21wb25lbnRzIHRo ZW1zZWx2ZXMsIHdoaWNoIG1ha2VzIGhhcmQgZm9yIHRoZSBEVCBtYWludGFpbmVycyB0bwpoYXZl IGEgZ2xvYmFsIHVuZGVyc3RhbmRpbmcgb2YgdGhlIHZpZGVvIHN1YnN5c3RlbS4KCkhhdmluZyB0 aGUgdG9wb2xvZ3kgZGVzY3JpYmVkIGluIHRoZSBtYXN0ZXIgZGV2aWNlIGlzIGNsZWFyZXIsIGV2 ZW4gaWYKaXQgaXMgY29tcGxleC4KCj4gQSBnaXZlbiBkZXZpY2UgY2FuIGJlIGludGVncmF0ZWQg aW4gYSB3aWRlIHZhcmlldHkgb2YgaGFyZHdhcmUgd2l0aCBkaWZmZXJlbnQgCj4gY29tcGxleGl0 aWVzLiBBIGRyaXZlciBjYW4ndCB0aHVzIGFsd2F5cyBhc3N1bWUgdGhhdCB0aGUgd2hvbGUgY29t cG9zaXRlIAo+IGRpc3BsYXkgZGV2aWNlIHdpbGwgbWF0Y2ggYSBnaXZlbiBjbGFzcyBvZiB0b3Bv bG9naWVzLiBUaGlzIGlzIGVzcGVjaWFsbHkgdHJ1ZSAKPiBmb3IgZW5jb2RlcnMgYW5kIGNvbm5l Y3RvcnMsIHRoZXkgY2FuIGJlIGhvb2tlZCB1cCBkaXJlY3RseSBhdCB0aGUgb3V0cHV0IG9mIGEg Cj4gdmVyeSBzaW1wbGUgZGlzcGxheSBjb250cm9sbGVyLCBvciBjYW4gYmUgcGFydCBvZiBhIHZl cnkgY29tcGxleCBncmFwaC4gCj4gRW5jb2RlciBhbmQgY29ubmVjdG9yIGRyaXZlcnMgY2FuJ3Qg YXNzdW1lIG11Y2ggYWJvdXQgaG93IHRoZXkgd2lsbCBiZSAKPiBpbnRlZ3JhdGVkLiBJIHdhbnQg dG8gYXZvaWQgYSBzaXR1YXRpb24gd2hlcmUgdXNpbmcgYW4gSERNSSBlbmNvZGVyIGFscmVhZHkg Cj4gc3VwcG9ydGVkIGluIG1haW5saW5lIHdpdGggYSBkaWZmZXJlbnQgU29DIHdpbGwgcmVzdWx0 IGluIGhhdmluZyB0byByZXdyaXRlIAo+IHRoZSBIRE1JIGVuY29kZXIgZHJpdmVyLgoKVGhlIHRk YTk5OHggY2hpcHMgYXJlIHNpbXBsZSBlbm91Z2ggZm9yIGJlaW5nIHVzZWQgaW4gbWFueSBib2Fy ZHM6IG9uZQp2aWRlbyBpbnB1dCBhbmQgb25lIHNlcmlhbCBkaWdpdGFsIG91dHB1dC4gSSBkb24n dCBzZWUgaW4gd2hpY2gKY2lyY3Vtc3RhbmNlIHRoZSBkcml2ZXIgc2hvdWxkIGJlIHJld3JpdHRl bi4KCj4gVGhlIHN0b3J5IGlzIGEgYml0IGRpZmZlcmVudCBmb3IgZGlzcGxheSBjb250cm9sbGVy cy4gV2hpbGUgdGhlIGhhcmR3YXJlIAo+IHRvcG9sb2d5IG91dHNpZGUgKGFuZCBzb21ldGltZXMg aW5zaWRlIGFzIHdlbGwpIG9mIHRoZSBTb0MgY2FuIHZhcnksIGEgZGlzcGxheSAKPiBjb250cm9s bGVyIG9mdGVuIGFwcHJveGltYXRlbHkgaW1wbGllcyBhIGdpdmVuIGxldmVsIG9mIGNvbXBsZXhp dHkuIEEgY2hlYXAgCj4gU29DIHdpdGggYSBzaW1wbGUgZGlzcGxheSBjb250cm9sbGVyIHdpbGwg bGlrZWx5IG5vdCBiZSB1c2VkIHRvIGRyaXZlIGEgNGsgCj4gZGlzcGxheSByZXF1aXJpbmcgNCBl bmNvZGVycyBydW5uaW5nIGluIHBhcmFsbGVsIGZvciBpbnN0YW5jZS4gV2hpbGUgSSBhbHNvIAo+ IHdhbnQgdG8gYXZvaWQgaGF2aW5nIHRvIG1ha2Ugc2lnbmlmaWNhbnQgY2hhbmdlcyB0byBhIGRp c3BsYXkgY29udHJvbGxlciAKPiBkcml2ZXIgd2hlbiB1c2luZyB0aGUgU29DIG9uIGEgZGlmZmVy ZW50IGJvYXJkLCBJIGJlbGlldmUgdGhlIHJlcXVpcmVtZW50IGNhbiAKPiBiZSBzbGlnaHRseSBy ZWxheGVkIGhlcmUsIGFuZCB0aGUgZGlzcGxheSBjb250cm9sbGVyIGRyaXZlcihzKSBjYW4gYXNz dW1lIG1vcmUgCj4gYWJvdXQgdGhlIGhhcmR3YXJlIHRvcG9sb2d5IHRoYW4gdGhlIGVuY29kZXIg ZHJpdmVycy4KCkkgZG9uJ3QgdGhpbmsgdGhhdCB0aGUgZGlzcGxheSBjb250cm9sbGVycyBvciB0 aGUgZW5jb2RlcnMgaGF2ZSB0byBrbm93CmFib3V0IHRoZSB0b3BvbG9neS4gVGhleSBoYXZlIHdl bGwta253b24gc3BlY2lmaWMgZnVuY3Rpb25zIGFuZCB0aGUgd2F5CnRoZXkgYXJlIGludGVyY29u bmVjdGVkIHNob3VsZCBub3QgaW1wYWN0IHRoZXNlIGZ1bmN0aW9ucy4gSWYgdGhhdAp3b3VsZCBi ZSB0aGUgY2FzZSwgdGhleSBzaG91bGQgb2ZmZXIgYSBwYXJ0aWN1bGFyIGNvbmZpZ3VyYXRpb24K aW50ZXJmYWNlIHRvIHRoZSBtYXN0ZXIgZHJpdmVyLgoKPiBJJ3ZlIGFza2VkIG15c2VsZiB3aGV0 aGVyIHdlIG5lZWRlZCBhIHNpbmdsZSwgb25lLXNpemUtZml0cy10aGVtLWFsbCBEVCAKPiByZXBy ZXNlbnRhdGlvbiBvZiB0aGUgaGFyZHdhcmUgdG9wb2xvZ3kuIFRoZSB2aWV3IG9mIHRoZSB3b3Js ZCBmcm9tIGFuIAo+IGV4dGVybmFsIGVuY29kZXIgcG9pbnQgb2YgdmlldyBtdXN0IG5vdCBkZXBl bmQgb24gdGhlIFNvQyBpdCBpcyBob29rZWQgdXAgdG8gCj4gKHRoaXMgd291bGQgcHJldmVudCB1 c2luZyBhIHNpbmdsZSBlbmNvZGVyIGRyaXZlciB3aXRoIG11bHRpcGxlIFNvQ3MpLCB3aGljaCAK PiBjYWxscyBmb3IgYXQgbGVhc3Qgc29tZSBmb3JtIG9mIHN0YW5kYXJkaXphdGlvbi4gVGhlIHRv cG9sb2d5IHJlcHJlc2VudGF0aW9uIAo+IG9uIHRoZSBkaXNwbGF5IGNvbnRyb2xsZXIgc2lkZSBt YXkgdmFyeSBmcm9tIGRpc3BsYXkgY29udHJvbGxlciB0byBkaXNwbGF5IAo+IGNvbnRyb2xsZXIs IGJ1dCBJIGJlbGlldmUgdGhpcyB3b3VsZCBqdXN0IHJlc3VsdCBpbiBjb2RlIGR1cGxpY2F0aW9u IGFuZCAKPiBoYXZpbmcgdG8gc29sdmUgdGhlIHNhbWUgcHJvYmxlbSBpbiBtdWx0aXBsZSBkcml2 ZXJzLiBGb3IgdGhvc2UgcmVhc29ucyBJIAo+IGJlbGlldmUgdGhhdCB0aGUgT0YgZ3JhcGggcHJv cG9zYWwgdG8gcmVwcmVzZW50IHRoZSBkaXNwbGF5IGhhcmR3YXJlIHRvcG9sb2d5IAo+IHdvdWxk IGJlIGEgZ29vZCBjaG9pY2UuIFRoZSBiaW5kaW5ncyBhcmUgZmxleGlibGUgZW5vdWdoIHRvIHJl cHJlc2VudCBib3RoIAo+IHNpbXBsZSBhbmQgY29tcGxleCBoYXJkd2FyZS4KCllvdXIgT0YgZ3Jh cGggcHJvcG9zYWwgaXMgcmF0aGVyIGNvbXBsZXgsIGFuZCBpdCBkb2VzIG5vdCBldmVuIGluZGlj YXRlCndoaWNoIHdheSB0aGUgZGF0YSBmbG93cy4KCj4gTm93LCBJIGRvbid0IHdhbnQgdG8gZm9y Y2UgYWxsIGRpc3BsYXkgZGV2aWNlIGRyaXZlcnMgdG8gaW1wbGVtZW50IGNvbXBsZXggCj4gY29k ZSB3aGVuIHRoZXkgb25seSBuZWVkIHRvIHN1cHBvcnQgc2ltcGxlIGhhcmR3YXJlIGFuZCBzaW1w bGUgaGFyZHdhcmUgCj4gdG9wb2xvZ2llcy4gTm90IG9ubHkgd291bGQgdGhhdCBiZSByaWdodGZ1 bGx5IHJlamVjdGVkLCBJIHdvdWxkIGJlIGFtb25nIHRoZSAKPiBvbmVzIG5hY2snaW5nIHRoYXQg YXBwcm9hY2guIE15IG9waW5pb24gaXMgdGhhdCB0aGlzIGNhbGxzIGZvciB0aGUgY3JlYXRpb24g b2YgCj4gaGVscGVycyB0byBoYW5kbGUgY29tbW9uIGNhc2VzLiBTZXZlcmFsIChwb3NzaWJseSBt YW55KSBkaXNwbGF5IHN5c3RlbXMgb25seSAKPiBuZWVkIChvciB3YW50KSB0byBzdXBwb3J0IGxp bmVhciBwaXBlbGluZXMgYXQgdGhlaXIgb3V0cHV0KHMpLCBtYWRlIG9mIGEgCj4gc2luZ2xlIGVu Y29kZXIgYW5kIGEgc2luZ2xlIGNvbm5lY3Rvci4gVGhlcmUncyBubyBwb2ludCBpbiBkdXBsaWNh dGluZyBEVCAKPiBwYXJzaW5nIG9yIGVuY29kZXIvY29ubmVjdG9yIGluc3RhbnRpYXRpb24gY29k ZSBpbiBzZXZlcmFsIGRyaXZlcnMgaW4gdGhhdCAKPiBjYXNlIHdoZXJlIGhlbHBlcnMgY291bGQg YmUgcmV1c2VkLiBTZXZlcmFsIHNldHMgb2YgaGVscGVycyBjb3VsZCBzdXBwb3J0IAo+IGRpZmZl cmVudCBraW5kcyBvZiB0b3BvbG9naWVzLCB3aXRoIHRoZSBkcml2ZXIgYXV0aG9yIHNlbGVjdGlu ZyBhIHNldCBvZiAKPiBoZWxwZXJzIGRlcGVuZGluZyBvbiB0aGUgZXhwZWN0ZWQgaGFyZHdhcmUg dG9wb2xvZ3kgY29tcGxleGl0eS4KCkkgZG9uJ3QgbGlrZSB0aGlzICdoZWxwZXInIG5vdGlvbi4g U2VlIGJlbG93LgoKPiBXZSBhbHNvIG5lZWQgdG8gZGVjaWRlIG9uIHdobyAoYXMgaW4gd2hpY2gg ZHJpdmVyKSB3aWxsIGJlIHJlc3BvbnNpYmxlIGZvciAKPiBiaW5kaW5nIGFsbCBkZXZpY2VzIHRv Z2V0aGVyLiBBcyBEUk0gZXhwb3NlcyBhIHNpbmdsZSBkZXZpY2UgdG8gdXNlcnNwYWNlLCAKPiB0 aGVyZSBuZWVkcyB0byBiZSBhIHNpbmdsZSBkcml2ZXIgdGhhdCB3aWxsIHBlcmZvcm0gZnJvbnQg bGluZSBoYW5kbGluZyBvZiB0aGUgCj4gdXNlcnNwYWNlIEFQSSBhbmQgZGVsZWdhdGUgY2FsbHMg dG8gdGhlIG90aGVyIGRyaXZlcnMgaW52b2x2ZWQuIEkgYmVsaWV2ZSBpdCAKPiB3b3VsZCBiZSBs b2dpY2FsIGZvciB0aGF0IGRyaXZlciB0byBhbHNvIGJlIGluIGNoYXJnZSBvZiBiaW5kaW5ncyB0 aGUgCj4gY29tcG9uZW50cyB0b2dldGhlciwgYXMgaXQgd2lsbCBiZSB0aGUgZHJpdmVyIHRoYXQg ZGVsZWdhdGUgY2FsbHMgdG8gdGhlIAo+IGNvbXBvbmVudHMuIEZvciBhIHNpbWlsYXIgcmVhc29u IEkgYWxzbyBiZWxpZXZlIHRoYXQgdGhhdCBkcml2ZXIgc2hvdWxkIGFsc28gCj4gYmUgdGhlIG9u ZSBpbiBjb250cm9sIG9mIGNyZWF0aW5nIERSTSBlbmNvZGVycyBhbmQgRFJNIGNvbm5lY3RvcnMu IFRoZSAKPiBjb21wb25lbnQgZHJpdmVycyBoYXZpbmcgb25seSBhIG5hcnJvdyB2aWV3IG9mIHRo ZSBkZXZpY2UgdGhleSBoYW5kbGUsIHRoZXkgCj4gY2FuJ3QgcGVyZm9ybSB0aGF0IHRhc2sgaW4g YSBnZW5lcmljIHdheSBhbmQgd291bGQgdGh1cyBnZXQgdGllZCB0byBzcGVjaWZpYyAKPiBtYXN0 ZXIgZHJpdmVycyBiZWNhdXNlIG9mIHRoZSBhc3N1bXB0aW9ucyB0aGV5IHdvdWxkIG1ha2UuCgpJ IGRvbid0IHNlZSB3aHkgdGhlIGVuY29kZXJzIGFuZCBjb25uZWN0b3JzIHNob3VsZCBiZSBjcmVh dGVkIGJ5IHNvbWUKZXh0ZXJuYWwgZW50aXR5LiBUaGV5IGFyZSBkZWNsYXJlZCBpbiB0aGUgRFQg b3IgY3JlYXRlZCBieSB0aGUgYm9hcmQKaW5pdCwgc28sIHRoZXkgZXhpc3QgYXQgc3lzdGVtIHN0 YXJ0dXAgdGltZS4KCj4gV2hldGhlciB0aGUgbWFzdGVyIGRyaXZlciBpcyB0aGUgQ1JUQyBkcml2 ZXIgb3IgYSBzZXBhcmF0ZSBkcml2ZXIgdGhhdCBiaW5kcyAKPiBzdGFuZGFsb25lIENSVENzLCBj b25uZWN0b3JzIGFuZCBlbmNvZGVycyBkb2Vzbid0IGluIG15IG9waW5pb24gY2hhbmdlIG15IAo+ IGFib3ZlIGNvbmNsdXNpb25zLiBTb21lIFNvQ3MgY291bGQgdXNlIGEgc2ltcGxlLXZpZGVvLWNh cmQgZHJpdmVyIGxpa2UgdGhlIG9uZSAKPiB5b3UndmUgcHJvcG9zZWQsIGFuZCBvdGhlcnMgY291 bGQgaW1wbGVtZW50IGEgZGlzcGxheSBjb250cm9sbGVyIGRyaXZlciB0aGF0IAo+IHdvdWxkIHBl cmZvcm0gdGhlIHNhbWUgdGFza3MgZm9yIG1vcmUgY29tcGxleCBwaXBlbGluZXMgaW4gYWRkaXRp b24gdG8gCj4gY29udHJvbGxpbmcgdGhlIGRpc3BsYXkgY29udHJvbGxlcidzIENSVEMocykuIFRo ZSBzaW1wbGUtdmlkZW8tY2FyZCBkcml2ZXIgCj4gd291bGQgcGVyZm9ybSB0aGF0IHNhbWUgdGFz a3MgYXMgdGhlIGhlbHBlcnMgSSd2ZSBwcmV2aW91c2x5IHRhbGtlZCBhYm91dCwgc28gCj4gdGhl IHR3byBzb2x1dGlvbnMgYXJlIHByZXR0eSBzaW1pbGFyLCBhbmQgSSBkb24ndCBzZWUgbXVjaCBh ZGRlZCB2YWx1ZSBpbiB0aGUgCj4gZ2VuZXJhbCBjYXNlIGluIGhhdmluZyBhIHNpbXBsZS12aWRl by1jYXJkIGRyaXZlciBjb21wYXJlZCB0byB1c2luZyBoZWxwZXJzIGluIAo+IHRoZSBkaXNwbGF5 IGNvbnRyb2xsZXIgZHJpdmVyLgoKSW4gZmFjdCwgSSB3b25kZXIgd2h5IHRoZXJlIGlzIG5vdCBv bmx5IG9uZSBEUk0gZHJpdmVyLiBUaGUgZ2xvYmFsCmxvZ2ljIGlzIGFsd2F5cyB0aGUgc2FtZSwg YW5kLCBhY3R1YWxseSwgaXQgaXMgZHVwbGljYXRlZCBpbiBlYWNoCnNwZWNpZmljIGRyaXZlci4g SSBoYWQgdGhpcyBhcHByb2FjaCBpbiB0aGUgZ3NwY2EgZHJpdmVyOiBvbmUgbWFpbgpkcml2ZXIg YW5kIG1hbnkgc3BlY2lmaWMgc3ViZHJpdmVycy4gT25jZSwgSSBldmVuIGhhZCB0aGUgaWRlYSB0 bwpjb252ZXJ0IHlvdXIgVVZDIGRyaXZlciBpbnRvIGEgZ3NwY2Egc3ViZHJpdmVyLCBidXQsIGF0 IHRoaXMgdGltZSwgSQpoYWQgdG9vIG11Y2ggd29yayB3aXRoIHRoZSBvdGhlciB3ZWJjYW1zLiBO ZXZlcnRoZWxlc3MsIGl0IHdvdWxkIGhhdmUKYmVlbiBlYXN5OiBhbGwgdGhlIHJlcXVpcmVkIGlu dGVyZmFjZXMgd2VyZSAoYXJlIHN0aWxsKSBwcmVzZW50IGluIHRoZQptYWluIGdzcGNhIGRyaXZl ci4KCkluIHRoZSBzYW1lIG9yZGVyIG9mIGlkZWEsIHRoZXJlIGlzIG9ubHkgb25lIEFMU0EgU29D IGRyaXZlciwgYW5kIHlvdQpjYW5ub3Qgc2F5IHRoYXQgdGhlIGF1ZGlvIHRvcG9sb2d5IGlzIGxl c3MgY29tcGxleCB0aGFuIHRoZSB2aWRlbyBvbmUuCgpXaGVuIHRoaW5raW5nIGFib3V0IHRoZSB1 bmlxdWUgRFJNIGRyaXZlciwgdGhlcmUgd291bGQgbm8gaGVscGVyIGFueW1vcmU6CnRoZSBkcml2 ZXIgd291bGQgb2ZmZXIgdG8gZWFjaCBjb21wb25lbnQgdGhlIGZ1bmN0aW9ucyB0aGV5IG5lZWQg Zm9yCndvcmtpbmcgY29ycmVjdGx5LgoKPiBUaGUgcG9pbnQgdGhhdCBtYXR0ZXJzIHRvIG1lIGlz IHRoYXQgZW5jb2RlcnMgRFQgYmluZGluZ3MgYW5kIGRyaXZlcnMgbXVzdCBiZSAKPiBpZGVudGlj YWwgcmVnYXJkbGVzcyBvZiB3aGV0aGVyIHRoZSBtYXN0ZXIgZHJpdmVyIGlzIHRoZSBkaXNwbGF5 IGNvbnRyb2xsZXIgCj4gZHJpdmVyIG9yIGEgZHJpdmVyIGZvciBhIGxvZ2ljYWwgY29tcG9zaXRl IGRldmljZS4gRm9yIGFsbCB0aG9zZSByZWFzb25zIHdlIAo+IHNob3VsZCB1c2UgdGhlIE9GIGdy YXBoIERUIGJpbmRpbmdzIGZvciB0aGUgc2ltcGxlLXZpZGVvLWNhcmQgZHJpdmVyIHNob3VsZCB3 ZSAKPiBkZWNpZGUgdG8gaW1wbGVtZW50IGl0LCBhbmQgd2Ugc2hvdWxkIGNyZWF0ZSBEUk0gZW5j b2RlcnMgYW5kIGNvbm5lY3RvcnMgaW4gCj4gdGhlIG1hc3RlciBkcml2ZXIsIG5vdCBpbiB0aGUg ZW5jb2RlciBkcml2ZXJzLgoKU29ycnksIGJ1dCBJIHRoaW5rIHRoYXQgYSBEUk0gZW5jb2RlciBp cyB0aGUgaGFyZHdhcmUgZW5jb2Rlci4gV2hhdAplbHNlIGNvdWxkIGl0IGJlPyBBdCBpbml0aWFs aXphdGlvbiB0aW1lLCB0aGUgbWFzdGVyIGRyaXZlciBoYXMgb25seSB0bwptYW5hZ2UgdGhlIHJl bGF0aW9uIGJldHdlZW4gdGhlIHZpZGVvIGNvbXBvbmVudHMuIEl0IGhhcyBub3QgdG8gY3JlYXRl CmVudGl0aWVzIHdoaWNoIGFscmVhZHkgZXhpc3QuIFNvIG15IHByZWZlcnJlZCBzY2hlbWUgaXM6 CgotIGF0IHN0YXJ0aW5nIHRpbWUsIGVhY2ggdmlkZW8gY29tcG9uZW50IGluaXRpYWxpemVzIGl0 cyBzb2Z0d2FyZSBhbmQKICBoYXJkd2FyZSwgYW5kIHRoZW4sIHBsdWdzIGl0c2VsZiBpbnRvIHRo ZSBEUk0gZHJpdmVyLgoKLSBmcm9tIHRoZSBjZW50cmFsIHZpZGVvIHRvcG9sb2d5LCB0aGUgRFJN IGRldmljZSB3YWl0cyBmb3IgYWxsCiAgY29tcG9uZW50cyB0byBiZSBwbHVnZ2VkIGFuZCB0aGVu IGl0IGxpbmtzIHRoZSBjb21wb25lbnRzIHRvZ2V0aGVyLAogIGNyZWF0aW5nIHRoZSB1c2VyJ3Mg dmlldyBvZiB0aGUgc3lzdGVtLgoKSWYgeW91IGNyZWF0ZSB0aGUgZW5jb2RlcnMgYW5kIGNvbm5l Y3RvcnMgaW4gdGhlIERSTSBtYXN0ZXIsIHRoZQpoYXJkd2FyZSBlbmNvZGVyIGFuZCBjb25uZWN0 b3IgZGV2aWNlcyBhcmUganVzdCBsaWtlIHpvbWJpZXMuIFRoZXkgbXVzdApwdXQgc29tZXRoaW5n IGluIHRoZSBzeXN0ZW0gdG8gc2F5IHRoZXkgYXJlIGhlcmUgYW5kIHRoZXkgbXVzdCBhbHNvCmhh dmUgYSBjYWxsYmFjayBmdW5jdGlvbiBmb3IgdGhlbSB0byBrbm93IHRvIHdoaWNoIHZpZGVvIHN1 YnN5c3RlbSB0aGV5CmJlbG9uZy4KCk9uIHRoZSBjb250cmFyeSwgaWYgdGhlIERSTSBlbmNvZGVy cyBhbmQgY29ubmVjdG9ycyBhcmUgY3JlYXRlZCBieSB0aGUKaGFyZHdhcmUgZGV2aWNlcywgdGhl eSBhcmUgaW1tZWRpYXRlbHkgYWxpdmUgYW5kIG9wZXJhdGlvbmFsLgoKLS0gCktlbiBhciBjJ2hl bnRhw7EJfAkgICAgICAqKiBCcmVpemggaGEgTGludXggYXRhdiEgKioKSmVmCQl8CQlodHRwOi8v bW9pbmVqZi5mcmVlLmZyLwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3Rv cC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1k ZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp2-g21.free.fr ([212.27.42.2]:51107 "EHLO smtp2-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751683AbaC0LeQ convert rfc822-to-8bit (ORCPT ); Thu, 27 Mar 2014 07:34:16 -0400 Date: Thu, 27 Mar 2014 12:34:49 +0100 From: Jean-Francois Moine To: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org, Russell King , Rob Clark , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Subject: Re: [PATCH RFC v2 2/6] drm/i2c: tda998x: Move tda998x to a couple encoder/connector Message-ID: <20140327123449.65584e93@armhf> In-Reply-To: <6885089.l87kb3TNcV@avalon> References: <1458827.cQ6aDWdh1W@avalon> <20140325165548.0065b639@armhf> <6885089.l87kb3TNcV@avalon> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laurent, On Wed, 26 Mar 2014 18:33:09 +0100 Laurent Pinchart wrote: > That could work in your case, but I don't really like that. > > We need to describe the hardware topology, that might be the only point we all > agree on. There are various hardware topologies we need to support with > different levels of complexity, and several ways to describe them, also with a > wide range of complexities and features. > > The more complex the hardware topology, the more complex its description needs > to be, and the more complex the code that will parse the description and > handle the hardware will be. I don't think there's any doubt here. But also, the simpler is the topology and the simpler should be its description. In your approach, the connections between endpoints are described in the components themselves, which makes hard for the DT maintainers to have a global understanding of the video subsystem. Having the topology described in the master device is clearer, even if it is complex. > A given device can be integrated in a wide variety of hardware with different > complexities. A driver can't thus always assume that the whole composite > display device will match a given class of topologies. This is especially true > for encoders and connectors, they can be hooked up directly at the output of a > very simple display controller, or can be part of a very complex graph. > Encoder and connector drivers can't assume much about how they will be > integrated. I want to avoid a situation where using an HDMI encoder already > supported in mainline with a different SoC will result in having to rewrite > the HDMI encoder driver. The tda998x chips are simple enough for being used in many boards: one video input and one serial digital output. I don't see in which circumstance the driver should be rewritten. > The story is a bit different for display controllers. While the hardware > topology outside (and sometimes inside as well) of the SoC can vary, a display > controller often approximately implies a given level of complexity. A cheap > SoC with a simple display controller will likely not be used to drive a 4k > display requiring 4 encoders running in parallel for instance. While I also > want to avoid having to make significant changes to a display controller > driver when using the SoC on a different board, I believe the requirement can > be slightly relaxed here, and the display controller driver(s) can assume more > about the hardware topology than the encoder drivers. I don't think that the display controllers or the encoders have to know about the topology. They have well-knwon specific functions and the way they are interconnected should not impact these functions. If that would be the case, they should offer a particular configuration interface to the master driver. > I've asked myself whether we needed a single, one-size-fits-them-all DT > representation of the hardware topology. The view of the world from an > external encoder point of view must not depend on the SoC it is hooked up to > (this would prevent using a single encoder driver with multiple SoCs), which > calls for at least some form of standardization. The topology representation > on the display controller side may vary from display controller to display > controller, but I believe this would just result in code duplication and > having to solve the same problem in multiple drivers. For those reasons I > believe that the OF graph proposal to represent the display hardware topology > would be a good choice. The bindings are flexible enough to represent both > simple and complex hardware. Your OF graph proposal is rather complex, and it does not even indicate which way the data flows. > Now, I don't want to force all display device drivers to implement complex > code when they only need to support simple hardware and simple hardware > topologies. Not only would that be rightfully rejected, I would be among the > ones nack'ing that approach. My opinion is that this calls for the creation of > helpers to handle common cases. Several (possibly many) display systems only > need (or want) to support linear pipelines at their output(s), made of a > single encoder and a single connector. There's no point in duplicating DT > parsing or encoder/connector instantiation code in several drivers in that > case where helpers could be reused. Several sets of helpers could support > different kinds of topologies, with the driver author selecting a set of > helpers depending on the expected hardware topology complexity. I don't like this 'helper' notion. See below. > We also need to decide on who (as in which driver) will be responsible for > binding all devices together. As DRM exposes a single device to userspace, > there needs to be a single driver that will perform front line handling of the > userspace API and delegate calls to the other drivers involved. I believe it > would be logical for that driver to also be in charge of bindings the > components together, as it will be the driver that delegate calls to the > components. For a similar reason I also believe that that driver should also > be the one in control of creating DRM encoders and DRM connectors. The > component drivers having only a narrow view of the device they handle, they > can't perform that task in a generic way and would thus get tied to specific > master drivers because of the assumptions they would make. I don't see why the encoders and connectors should be created by some external entity. They are declared in the DT or created by the board init, so, they exist at system startup time. > Whether the master driver is the CRTC driver or a separate driver that binds > standalone CRTCs, connectors and encoders doesn't in my opinion change my > above conclusions. Some SoCs could use a simple-video-card driver like the one > you've proposed, and others could implement a display controller driver that > would perform the same tasks for more complex pipelines in addition to > controlling the display controller's CRTC(s). The simple-video-card driver > would perform that same tasks as the helpers I've previously talked about, so > the two solutions are pretty similar, and I don't see much added value in the > general case in having a simple-video-card driver compared to using helpers in > the display controller driver. In fact, I wonder why there is not only one DRM driver. The global logic is always the same, and, actually, it is duplicated in each specific driver. I had this approach in the gspca driver: one main driver and many specific subdrivers. Once, I even had the idea to convert your UVC driver into a gspca subdriver, but, at this time, I had too much work with the other webcams. Nevertheless, it would have been easy: all the required interfaces were (are still) present in the main gspca driver. In the same order of idea, there is only one ALSA SoC driver, and you cannot say that the audio topology is less complex than the video one. When thinking about the unique DRM driver, there would no helper anymore: the driver would offer to each component the functions they need for working correctly. > The point that matters to me is that encoders DT bindings and drivers must be > identical regardless of whether the master driver is the display controller > driver or a driver for a logical composite device. For all those reasons we > should use the OF graph DT bindings for the simple-video-card driver should we > decide to implement it, and we should create DRM encoders and connectors in > the master driver, not in the encoder drivers. Sorry, but I think that a DRM encoder is the hardware encoder. What else could it be? At initialization time, the master driver has only to manage the relation between the video components. It has not to create entities which already exist. So my preferred scheme is: - at starting time, each video component initializes its software and hardware, and then, plugs itself into the DRM driver. - from the central video topology, the DRM device waits for all components to be plugged and then it links the components together, creating the user's view of the system. If you create the encoders and connectors in the DRM master, the hardware encoder and connector devices are just like zombies. They must put something in the system to say they are here and they must also have a callback function for them to know to which video subsystem they belong. On the contrary, if the DRM encoders and connectors are created by the hardware devices, they are immediately alive and operational. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/