Hi Rob and Sascha, On Wednesday 21 August 2013 08:22:59 Rob Clark wrote: > On Wed, Aug 21, 2013 at 3:09 AM, Sascha Hauer wrote: > > 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? Many chips and IP cores in KMS and V4L2 pipelines are pretty simple, and the bulk of the driver is just an API implementation. Register poking is usually a small percentage of the code (that's of course not the case for the more complex IP cores, but those are usually not shared). > >> 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. The point is I believe they will become less and less exceptional over time. One might argue that CDF is a revolutionary approach as opposed to an evolutionary approach, but looking at the future I believe we'll need a much bigger disruption if we wait too long. > >> > 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. Don't get me wrong, drm_bridge is an interesting idea, but I think it's a bit limited. > >> >> 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. > > oh sure, this is why I'd be absolutely against exposing this to userspace > currently.. At least we agree on that, good :-) > But my only point here was that an in-drm framework, all the fix-ups due to > a framework change are sorted out before Dave sends his pull req to linus. > We do this on a semi-regular basis already in drm already. I understand. But DRM drivers already depend on lots of infrastructure code that is not part of DRM (you could of course argue that it's not a valid reason to add one more :-)). > Anyways, not a show-stopper thing.. just (in my mind) one additional > inconvenience (and not really the biggest one) about having the "framework" > outside of drm. I'm more concerned about just having the "display entity" > code at a layer below the helpers and property api's that I'd like to use in > drm. I very much like the KMS property API (there's of course always room for improvements), we're actually thinking about a property API for V4L2 as well. My concern is that we need something shareable between subsystems, and the KMS version is a bit too tied to DRM/KMS for that purpose. For instance struct drm_property inherits struct drm_mode_object, and thus requires a DRM device to allocate IDs. This is the kind of really low-level issues that we will need to sort out one way or the other. If we can't require a V4L2 driver to implement a drm_device, or the other around (and I believe it would be unreasonable to do so), we will need a neutral middle-ground. > And just to be clear, part of my negative experience about this is the > omapdss/omapdrm split. I just see cfd outside of drm as encouraging > others to make the same mistake. I absolutely agree that we should not repeat that mistake. We hopefully have a bit more experience now, and we should catch this kind of issue during code review in the worst case. > >> 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. > > right, which is why I want to "optimize" for this case by having everything > effected by evolving the framework merged via drm. > > >> 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. > > yes, DT binding are orthogonal. This is something that should work > for BIOS, ACPI, DT, fex (just kidding), :-D > etc. I mean there might be some DT helpers, but that should be kind of on > the side. > > If DT binding are really to be OS independent, we probably need some better > way to deal w/ collecting up DT nodes for one (userspace visible) driver (or > cases where driver <-> DT is not 1<->1). But that is really an entirely > different topic, or at least not applicable to where the "framework" lives. I pretty much agree, DT bindings won't depend on whether we upstream the code to drivers/video/, drivers/gpu/ or drivers/media/. However, it's easy to design DT bindings with a particular framework implementation in mind and use shortcuts that make the bindings difficult to use should a different framework be implemented. We need to be careful about this. On the upside we now have more experienced DT bindings reviewers than a couple of years ago, I'm thus pretty optimistic. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
