Re: [PATCH v3 09/13] drm/sun4i: backend: Wire in the frontend

[Maxime Ripard]

Hi,

On Wed, Jan 17, 2018 at 09:46:13PM +0800, Chen-Yu Tsai wrote:
> > +       for_each_available_child_of_node(port, ep) {
> > +               remote = of_graph_get_remote_port_parent(ep);
> > +               if (!remote)
> > +                       continue;
> > +
> > +               /* does this node match any registered engines? */
> > +               list_for_each_entry(frontend, &drv->frontend_list, list) {
> > +                       if (remote == frontend->node) {
> > +                               of_node_put(remote);
> > +                               of_node_put(port);
> > +                               return frontend;
> > +                       }
> > +               }
> 
> I assume that since you are only enabling support on the A33, you are
> not considering dual display pipeline cases. We have cross connections
> in the of graph for those situations, i.e. frontend0 can output to both
> backend0 and backend1, as described in the following link:
> 
> http://elixir.free-electrons.com/linux/v4.15-rc8/source/arch/arm/boot/dts/sun6i-a31.dtsi#L1177
> 
> So with your code, both backends would end up using frontend0, and
> if both display pipelines are active, one would potentially step on
> the other.
> 
> Instead, have the frontend store its ID in its data structure, and
> have the backend look through the list of frontends to find a match
> for the backend's ID. This is what we already do for engine <-> TCON:
> 
> http://elixir.free-electrons.com/linux/v4.15-rc8/source/drivers/gpu/drm/sun4i/sun4i_tcon.c#L583
> 
> Or just leave a TODO note and I can do it later. :)

We discussed it on IRC, and yes, I don't have a test setup right now
with two pipelines being used, and there's a few changes that need to
be made for it to operate properly on the A10/A20. That's why I only
added the A33 in the supported list at the moment.

I'll add a TODO note

> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai <wens@xxxxxxxx>

Thanks!
Maxime

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

Attachment: signature.asc
Description: PGP signature