Re: [PATCH/RFC v3 00/19] Common Display Framework

[Sascha Hauer <s.hauer@pengutronix.de>]

On Tue, Aug 20, 2013 at 02:40:55PM -0400, Rob Clark wrote:
> On Tue, Aug 20, 2013 at 11:24 AM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> 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 |