Date:   Fri, 9 Jun 2017 16:46:49 +0200
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     icenowy@...c.io
Cc:     devicetree@...r.kernel.org,
        Jernej Škrabec <jernej.skrabec@...l.net>,
        linux-sunxi@...glegroups.com, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, Chen-Yu Tsai <wens@...e.org>,
        Rob Herring <robh+dt@...nel.org>, linux-clk@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 03/11] drm: sun4i: ignore swapped mixer<->tcon
 connection for DE2

On Thu, Jun 08, 2017 at 01:01:53PM +0800, icenowy@...c.io wrote:
> 在 2017-06-07 22:38，Maxime Ripard 写道：
> > On Wed, Jun 07, 2017 at 06:01:02PM +0800, Icenowy Zheng wrote:
> > > >I have no idea what this is supposed to be doing either.
> > > >
> > > >I might be wrong, but I really feel like there's a big mismatch
> > > >between your commit log, and what you actually implement.
> > > >
> > > >In your commit log, you should state:
> > > >
> > > >A) What is the current behaviour
> > > >B) Why that is a problem
> > > >C) How do you address it
> > > >
> > > >And you don't.
> > > >
> > > >However, after discussing it with Chen-Yu, it seems like you're trying
> > > >to have all the mixers probed before the TCONs. If that is so, there's
> > > >nothing specific to the H3 here, and we also have the same issue on
> > > >dual-pipeline DE1 (A10, A20, A31). Chen-Yu worked on that a bit, but
> > > >the easiest solution would be to move from a DFS algorithm to walk
> > > >down the graph to a BFS one.
> > > >
> > > >That way, we would add all mixers first, then the TCONs, then the
> > > >encoders, and the component framework will probe them in order.
> > > 
> > > No. I said that they're swappable, however, I don't want to
> > > implement the swap now, but hardcode 0-0 1-1 connection.
> > 
> > We're on the same page, it's definitely not what I was mentionning
> > here. This would require a significant rework, and the usecase is
> > still unclear for now.
> > 
> > > However, as you and Chen-Yu said, device tree should reflect the
> > > real hardware, there will be bonus endpoints for the swapped
> > > connection.
> > 
> > If by bonus you mean connections from mixer 0 to tcon 1 and mixer 1 to
> > tcon 0, then yes, we're going to need it.
> > 
> > > What I want to do is to ignore the bonus connection, in order to
> > > prevent them from confusing the code.
> > > 
> > > If you just change the bind sequence, I think it cannot be
> > > prevented that wrong connections will be bound.
> > 
> > This is where I don't follow you anymore. The component framework
> > doesn't list connections but devices. The swapped connections do not
> > matter here, we have the same set of devices: mixer0, mixer1, tcon0
> > and tcon1.
> > 
> > The thing that does change with your patch is that before, the binding
> > sequence would have been mixer0, tcon0, tcon1, mixer1. With your
> > patch, it's mixer0, tcon0, mixer1, tcon1.
> > 
> > So, again, stating what issue you were seeing before making this patch
> > would be very helpful to see what you're trying to do / fix.
> 
> So maybe I can drop the forward search (searching output) code, and keep
> only the backward search (search input) code in TCON?
> 
> Forward search code is only used when binding, but backward search is used
> for TCON to find connected mixer.

It is hard to talk about a solution, when it's not clear what the
issue is.

So please state 
> > > >A) What is the current behaviour
> > > >B) Why that is a problem
> > > >C) How do you address it

We'll talk about a solution once this is done.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)

Powered by blists - more mailing lists