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

From: Maxime Ripard
Date: Thu Jan 18 2018 - 02:25:28 EST


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