From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sascha Hauer Subject: Re: [PATCH/RFC v3 00/19] Common Display Framework Date: Wed, 21 Aug 2013 09:09:47 +0200 Message-ID: <20130821070947.GM31036@pengutronix.de> References: <1376068510-30363-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <1599910.F5touEILFq@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [92.198.50.35]) by gabe.freedesktop.org (Postfix) with ESMTP id 152EBE5F6D for ; Wed, 21 Aug 2013 00:10:00 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Rob Clark Cc: Linux Fbdev development list , Benjamin Gaignard , "dri-devel@lists.freedesktop.org" , Jesse Barnes , Laurent Pinchart , Sebastien Guiriec , Laurent Pinchart , Tom Gall , Ragesh Radhakrishnan , Tomi Valkeinen , "linux-media@vger.kernel.org" , Stephen Warren , Mark Zhang , =?iso-8859-15?Q?St=E9phane?= Marchesin , Alexandre Courbot , Thomas Petazzoni , Sunil Joshi , Kyungmin Park Maxime Ripard List-Id: dri-devel@lists.freedesktop.org On Tue, Aug 20, 2013 at 02:40:55PM -0400, Rob Clark wrote: > On Tue, Aug 20, 2013 at 11:24 AM, Laurent Pinchart > wrote: > > Hi Rob, > > > >> Or maybe, put another way, I still think we should still optimize for the > >> common case. I mean I'm sure you *can* design hw that has an LVDS->DP bridge > >> in the capture path, and if you had something like an FPGA where that was > >> cheap to do maybe you even would (for fun). But if in the real world, there > >> are only one or two cases of actual hw using the same bridge in a capture > >> pipeline which is normally used in display pipelines, then duplicating some > >> small bit of code for that abnormal case if it makes things easier for the > >> common case, seems like a reasonable trade-off to me. > > > > That was my opinion as well until I started working on a Xilinx platform. > > There's dozens of IP cores there that are used in both capture and display > > pipelines. > > or maybe just some helper lib's to handling the register level programming? > > Anyways, I guess you can call it a "worse is better" thing, but if you > can get 90% of the desired effect for 10% of the work, take the > simpler solution. I don't think we should make things more layered or > more difficult for one exceptional case. > > > > Furthermore, FPGA display pipelines being made of individual IP cores, we need > > a way to write one driver per IP core and make them all interact. The recently > > proposed drm_bridge is one possible solution to this issue (and to a different > > but similar issue that trigerred the development of drm_bridge), but it's in > > my opinion not generic enough. We're facing several problems that are related, > > it would really be a shame not to solve them with a good enough framework. > > well, I've been working on DSI panel support in msm drm, and also been > looking at the patches to add DSI support in i915. And the panel > interface ends up looking basically like the drm_bridge interface. > Perhaps the only thing not generic there is the name. > > >> I mean, take a DSI panel driver, for example.. anything but a trivial panel > >> driver, you are going to want to expose some custom properties on the > >> connector for controlling non-standard features. So you end up with both > >> cdf_foo for the common part, and drm_foo for gluing in the properties. And > >> now these two parts which otherwise would be one, end up having to stay in > >> sync merged through different trees, etc. It seems a lot like making my life > >> more difficult for a fairly hypothetical gain ;-) > > > > The DSI panel driver should not be split into two parts. It should implement a > > CDF entity, any glue needed for DRM will not be part of the panel driver. > > right, but that glue ends up needing to be *somewhere*, which makes it > the 2nd part. > > >> Or, take an hdmi or DP bridge. To use any of the common > >> infrastructure/helpers in drm, you end up implementing a generic interface, > >> where both the producer and consumer is inside drm. Which just seems a bit > >> pointless and extra hoops to jump through. Especially when we discover that > >> we need to extend/enhance the common interface outside of drm to make it > >> work. (I'm thinking of display_entity_control_ops::get_modes() here, but I'm > >> sure there are more examples.) And I think we'll run into similar issues > >> with display_entity_control_ops::set_state(), since the on<->off sequencing > >> can get hairy when the upstream/downstream entity is fed a clk by the > >> downstream/upstream entity. And similarly, I think we'll go through a few > >> revisions of DSI panel/bus params before we have everything that everyone > >> needs. > > > > I don't see where needing multiple revisions of a patch set would be bad :-) > > I mean over multiple kernel revisions, as people start adding support > for new hw and run into limitations of the "framework". A framework can be changed, extended and fixed. In the end we talking about a completely in-kernel framework for which we do not have to maintain a stable API. > > We've already figured out that just having enable() and disable() is > not sufficient for displayport link training. I'm not sure what else > we'll discover. It's pretty much expected that other things will be discovered, but this will also happen with all sub-drm-driver frameworks we currently have. > > I'm not saying not to solve (most of these) problems.. I don't think > chaining multiple drm_bridge's is impossible, I can think of at least > two ways to implement it. But I think we should solve that as someone > is adding support for hw that uses this. This (a) lets us break > things up into smaller more incremental changes (drm_bridge is > *considerably* smaller than the current CDF patchset), and (b) avoids > us solving the wrong problems. > > btw, I think DT bindings is orthogonal to this discussion. Not really. The common display framework is about splitting the pipeline into its components and adding a common view to the components. It's CDF helper code that can translate a DT binding directly into a encoder pipeline. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |