| Message ID | 20180625120304.7543-11-jernej.skrabec@siol.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 25, 2018 at 8:02 PM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > Current "old" method to find engine worked pretty well for DE2. However, > it doesn't work when TCON TOP is between mixer (engine) and TCON. TCON > TOP has multiple input ports, but current engine search algorithm > expects only one. > > This can be fixed by first looking for output port id and selecting > matching input by subtracting 1 for the next round. This work even if > there is only one input and output. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 08747fc3ee71..264bcc43da11 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct device *dev, > */ > static struct sunxi_engine * > sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > - struct device_node *node) > + struct device_node *node, > + u32 port_id) > { > struct device_node *port, *ep, *remote; > struct sunxi_engine *engine = ERR_PTR(-EINVAL); > + u32 reg = 0; > > - port = of_graph_get_port_by_id(node, 0); > + port = of_graph_get_port_by_id(node, port_id); > if (!port) > return ERR_PTR(-EINVAL); > > @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > if (remote == engine->node) > goto out_put_remote; > > + /* > + * According to device tree binding input ports have even id > + * number and output ports have odd id. Since component with > + * more than one input and one output (TCON TOP) exits, correct > + * remote input id has to be calculated by subtracting 1 from > + * remote output id. If this for some reason can't be done, 0 > + * is used as input port id. > + */ You need to call of_node_put(port); to drop the reference to the original port. Otherwise, Reviewed-by: Chen-Yu Tsai <wens@csie.org> > + port = of_graph_get_remote_port(ep); > + if (!of_property_read_u32(port, "reg", ®) && reg > 0) > + reg -= 1; > + > /* keep looking through upstream ports */ > - engine = sun4i_tcon_find_engine_traverse(drv, remote); > + engine = sun4i_tcon_find_engine_traverse(drv, remote, reg); > > out_put_remote: > of_node_put(remote); > @@ -950,7 +964,7 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, > > /* Fallback to old method by traversing input endpoints */ > of_node_put(port); > - return sun4i_tcon_find_engine_traverse(drv, node); > + return sun4i_tcon_find_engine_traverse(drv, node, 0); > } > > static int sun4i_tcon_bind(struct device *dev, struct device *master, > -- > 2.18.0 >
Dne četrtek, 28. junij 2018 ob 04:06:52 CEST je Chen-Yu Tsai napisal(a): > On Mon, Jun 25, 2018 at 8:02 PM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > > Current "old" method to find engine worked pretty well for DE2. However, > > it doesn't work when TCON TOP is between mixer (engine) and TCON. TCON > > TOP has multiple input ports, but current engine search algorithm > > expects only one. > > > > This can be fixed by first looking for output port id and selecting > > matching input by subtracting 1 for the next round. This work even if > > there is only one input and output. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > --- > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..264bcc43da11 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct device > > *dev, > > > > */ > > > > static struct sunxi_engine * > > sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > > > > - struct device_node *node) > > + struct device_node *node, > > + u32 port_id) > > > > { > > > > struct device_node *port, *ep, *remote; > > struct sunxi_engine *engine = ERR_PTR(-EINVAL); > > > > + u32 reg = 0; > > > > - port = of_graph_get_port_by_id(node, 0); > > + port = of_graph_get_port_by_id(node, port_id); > > > > if (!port) > > > > return ERR_PTR(-EINVAL); > > > > @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv > > *drv, > > > > if (remote == engine->node) > > > > goto out_put_remote; > > > > + /* > > + * According to device tree binding input ports have even id > > + * number and output ports have odd id. Since component with > > + * more than one input and one output (TCON TOP) exits, correct > > + * remote input id has to be calculated by subtracting 1 from > > + * remote output id. If this for some reason can't be done, 0 > > + * is used as input port id. > > + */ > > You need to call > > of_node_put(port); > > to drop the reference to the original port. Thanks for noticing it. I guess I should send fix patch, since patches from drm-misc-next can't be dropped. Best regards, Jernej > Otherwise, > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > + port = of_graph_get_remote_port(ep); > > + if (!of_property_read_u32(port, "reg", ®) && reg > 0) > > + reg -= 1; > > + > > > > /* keep looking through upstream ports */ > > > > - engine = sun4i_tcon_find_engine_traverse(drv, remote); > > + engine = sun4i_tcon_find_engine_traverse(drv, remote, reg); > > > > out_put_remote: > > of_node_put(remote); > > > > @@ -950,7 +964,7 @@ static struct sunxi_engine > > *sun4i_tcon_find_engine(struct sun4i_drv *drv,> > > /* Fallback to old method by traversing input endpoints */ > > of_node_put(port); > > > > - return sun4i_tcon_find_engine_traverse(drv, node); > > + return sun4i_tcon_find_engine_traverse(drv, node, 0); > > > > } > > > > static int sun4i_tcon_bind(struct device *dev, struct device *master, > > > > -- > > 2.18.0
On Thu, Jun 28, 2018 at 06:48:50AM +0200, Jernej Škrabec wrote: > Dne četrtek, 28. junij 2018 ob 04:06:52 CEST je Chen-Yu Tsai napisal(a): > > On Mon, Jun 25, 2018 at 8:02 PM, Jernej Skrabec <jernej.skrabec@siol.net> > wrote: > > > Current "old" method to find engine worked pretty well for DE2. However, > > > it doesn't work when TCON TOP is between mixer (engine) and TCON. TCON > > > TOP has multiple input ports, but current engine search algorithm > > > expects only one. > > > > > > This can be fixed by first looking for output port id and selecting > > > matching input by subtracting 1 for the next round. This work even if > > > there is only one input and output. > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > --- > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- > > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..264bcc43da11 > > > 100644 > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct device > > > *dev, > > > > > > */ > > > > > > static struct sunxi_engine * > > > sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > > > > > > - struct device_node *node) > > > + struct device_node *node, > > > + u32 port_id) > > > > > > { > > > > > > struct device_node *port, *ep, *remote; > > > struct sunxi_engine *engine = ERR_PTR(-EINVAL); > > > > > > + u32 reg = 0; > > > > > > - port = of_graph_get_port_by_id(node, 0); > > > + port = of_graph_get_port_by_id(node, port_id); > > > > > > if (!port) > > > > > > return ERR_PTR(-EINVAL); > > > > > > @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv > > > *drv, > > > > > > if (remote == engine->node) > > > > > > goto out_put_remote; > > > > > > + /* > > > + * According to device tree binding input ports have even id > > > + * number and output ports have odd id. Since component with > > > + * more than one input and one output (TCON TOP) exits, correct > > > + * remote input id has to be calculated by subtracting 1 from > > > + * remote output id. If this for some reason can't be done, 0 > > > + * is used as input port id. > > > + */ > > > > You need to call > > > > of_node_put(port); > > > > to drop the reference to the original port. > > Thanks for noticing it. I guess I should send fix patch, since patches from > drm-misc-next can't be dropped. Yeah, please send additional patches for all the issues pointed out by Chen-Yu. Maxime
Dne četrtek, 28. junij 2018 ob 20:25:43 CEST je Maxime Ripard napisal(a): > On Thu, Jun 28, 2018 at 06:48:50AM +0200, Jernej Škrabec wrote: > > Dne četrtek, 28. junij 2018 ob 04:06:52 CEST je Chen-Yu Tsai napisal(a): > > > On Mon, Jun 25, 2018 at 8:02 PM, Jernej Skrabec > > > <jernej.skrabec@siol.net> > > > > wrote: > > > > Current "old" method to find engine worked pretty well for DE2. > > > > However, > > > > it doesn't work when TCON TOP is between mixer (engine) and TCON. > > > > TCON > > > > TOP has multiple input ports, but current engine search algorithm > > > > expects only one. > > > > > > > > This can be fixed by first looking for output port id and selecting > > > > matching input by subtracting 1 for the next round. This work even if > > > > there is only one input and output. > > > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > > --- > > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- > > > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..264bcc43da11 > > > > 100644 > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct device > > > > *dev, > > > > > > > > */ > > > > > > > > static struct sunxi_engine * > > > > sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > > > > > > > > - struct device_node *node) > > > > + struct device_node *node, > > > > + u32 port_id) > > > > > > > > { > > > > > > > > struct device_node *port, *ep, *remote; > > > > struct sunxi_engine *engine = ERR_PTR(-EINVAL); > > > > > > > > + u32 reg = 0; > > > > > > > > - port = of_graph_get_port_by_id(node, 0); > > > > + port = of_graph_get_port_by_id(node, port_id); > > > > > > > > if (!port) > > > > > > > > return ERR_PTR(-EINVAL); > > > > > > > > @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv > > > > *drv, > > > > > > > > if (remote == engine->node) > > > > > > > > goto out_put_remote; > > > > > > > > + /* > > > > + * According to device tree binding input ports have even id > > > > + * number and output ports have odd id. Since component with > > > > + * more than one input and one output (TCON TOP) exits, > > > > correct > > > > + * remote input id has to be calculated by subtracting 1 from > > > > + * remote output id. If this for some reason can't be done, 0 > > > > + * is used as input port id. > > > > + */ > > > > > > You need to call > > > > > > of_node_put(port); > > > > > > to drop the reference to the original port. > > > > Thanks for noticing it. I guess I should send fix patch, since patches > > from > > drm-misc-next can't be dropped. > > Yeah, please send additional patches for all the issues pointed out by > Chen-Yu. Of course. I hope this can be resolved till the end of the next week. After that, I will be away from PC for 2 weeks. Feel free to drop DT patches if you think that it will come too close to merge window. Best regards, Jernej
Dne petek, 29. junij 2018 ob 21:06:09 CEST je Jernej Škrabec napisal(a): > Dne četrtek, 28. junij 2018 ob 20:25:43 CEST je Maxime Ripard napisal(a): > > On Thu, Jun 28, 2018 at 06:48:50AM +0200, Jernej Škrabec wrote: > > > Dne četrtek, 28. junij 2018 ob 04:06:52 CEST je Chen-Yu Tsai napisal(a): > > > > On Mon, Jun 25, 2018 at 8:02 PM, Jernej Skrabec > > > > <jernej.skrabec@siol.net> > > > > > > wrote: > > > > > Current "old" method to find engine worked pretty well for DE2. > > > > > However, > > > > > it doesn't work when TCON TOP is between mixer (engine) and TCON. > > > > > TCON > > > > > TOP has multiple input ports, but current engine search algorithm > > > > > expects only one. > > > > > > > > > > This can be fixed by first looking for output port id and selecting > > > > > matching input by subtracting 1 for the next round. This work even > > > > > if > > > > > there is only one input and output. > > > > > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > > > --- > > > > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- > > > > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > > > > > 08747fc3ee71..264bcc43da11 > > > > > 100644 > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct > > > > > device > > > > > *dev, > > > > > > > > > > */ > > > > > > > > > > static struct sunxi_engine * > > > > > sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > > > > > > > > > > - struct device_node *node) > > > > > + struct device_node *node, > > > > > + u32 port_id) > > > > > > > > > > { > > > > > > > > > > struct device_node *port, *ep, *remote; > > > > > struct sunxi_engine *engine = ERR_PTR(-EINVAL); > > > > > > > > > > + u32 reg = 0; > > > > > > > > > > - port = of_graph_get_port_by_id(node, 0); > > > > > + port = of_graph_get_port_by_id(node, port_id); > > > > > > > > > > if (!port) > > > > > > > > > > return ERR_PTR(-EINVAL); > > > > > > > > > > @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct > > > > > sun4i_drv > > > > > *drv, > > > > > > > > > > if (remote == engine->node) > > > > > > > > > > goto out_put_remote; > > > > > > > > > > + /* > > > > > + * According to device tree binding input ports have even id > > > > > + * number and output ports have odd id. Since component with > > > > > + * more than one input and one output (TCON TOP) exits, > > > > > correct > > > > > + * remote input id has to be calculated by subtracting 1 > > > > > from > > > > > + * remote output id. If this for some reason can't be done, > > > > > 0 > > > > > + * is used as input port id. > > > > > + */ > > > > > > > > You need to call > > > > > > > > of_node_put(port); > > > > > > > > to drop the reference to the original port. > > > > > > Thanks for noticing it. I guess I should send fix patch, since patches > > > from > > > drm-misc-next can't be dropped. > > > > Yeah, please send additional patches for all the issues pointed out by > > Chen-Yu. > > Of course. I hope this can be resolved till the end of the next week. After > that, I will be away from PC for 2 weeks. Feel free to drop DT patches if > you think that it will come too close to merge window. Actually, can you drop it anyway? It needs a lot of changes and keeping HDMI working would need some effort. Best regards, Jernej
On Sun, Jul 01, 2018 at 09:09:47PM +0200, Jernej Škrabec wrote: > Dne petek, 29. junij 2018 ob 21:06:09 CEST je Jernej Škrabec napisal(a): > > Dne četrtek, 28. junij 2018 ob 20:25:43 CEST je Maxime Ripard napisal(a): > > > On Thu, Jun 28, 2018 at 06:48:50AM +0200, Jernej Škrabec wrote: > > > > Dne četrtek, 28. junij 2018 ob 04:06:52 CEST je Chen-Yu Tsai napisal(a): > > > > > On Mon, Jun 25, 2018 at 8:02 PM, Jernej Skrabec > > > > > <jernej.skrabec@siol.net> > > > > > > > > wrote: > > > > > > Current "old" method to find engine worked pretty well for DE2. > > > > > > However, > > > > > > it doesn't work when TCON TOP is between mixer (engine) and TCON. > > > > > > TCON > > > > > > TOP has multiple input ports, but current engine search algorithm > > > > > > expects only one. > > > > > > > > > > > > This can be fixed by first looking for output port id and selecting > > > > > > matching input by subtracting 1 for the next round. This work even > > > > > > if > > > > > > there is only one input and output. > > > > > > > > > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > > > > > --- > > > > > > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- > > > > > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > > > > > > 08747fc3ee71..264bcc43da11 > > > > > > 100644 > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > > > > > @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct > > > > > > device > > > > > > *dev, > > > > > > > > > > > > */ > > > > > > > > > > > > static struct sunxi_engine * > > > > > > sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, > > > > > > > > > > > > - struct device_node *node) > > > > > > + struct device_node *node, > > > > > > + u32 port_id) > > > > > > > > > > > > { > > > > > > > > > > > > struct device_node *port, *ep, *remote; > > > > > > struct sunxi_engine *engine = ERR_PTR(-EINVAL); > > > > > > > > > > > > + u32 reg = 0; > > > > > > > > > > > > - port = of_graph_get_port_by_id(node, 0); > > > > > > + port = of_graph_get_port_by_id(node, port_id); > > > > > > > > > > > > if (!port) > > > > > > > > > > > > return ERR_PTR(-EINVAL); > > > > > > > > > > > > @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct > > > > > > sun4i_drv > > > > > > *drv, > > > > > > > > > > > > if (remote == engine->node) > > > > > > > > > > > > goto out_put_remote; > > > > > > > > > > > > + /* > > > > > > + * According to device tree binding input ports have even id > > > > > > + * number and output ports have odd id. Since component with > > > > > > + * more than one input and one output (TCON TOP) exits, > > > > > > correct > > > > > > + * remote input id has to be calculated by subtracting 1 > > > > > > from > > > > > > + * remote output id. If this for some reason can't be done, > > > > > > 0 > > > > > > + * is used as input port id. > > > > > > + */ > > > > > > > > > > You need to call > > > > > > > > > > of_node_put(port); > > > > > > > > > > to drop the reference to the original port. > > > > > > > > Thanks for noticing it. I guess I should send fix patch, since patches > > > > from > > > > drm-misc-next can't be dropped. > > > > > > Yeah, please send additional patches for all the issues pointed out by > > > Chen-Yu. > > > > Of course. I hope this can be resolved till the end of the next week. After > > that, I will be away from PC for 2 weeks. Feel free to drop DT patches if > > you think that it will come too close to merge window. > > Actually, can you drop it anyway? It needs a lot of changes and keeping HDMI > working would need some effort. No, we can't drop it, unfortunately. Maxime
diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..264bcc43da11 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -791,12 +791,14 @@ static int sun4i_tcon_init_regmap(struct device *dev, */ static struct sunxi_engine * sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, - struct device_node *node) + struct device_node *node, + u32 port_id) { struct device_node *port, *ep, *remote; struct sunxi_engine *engine = ERR_PTR(-EINVAL); + u32 reg = 0; - port = of_graph_get_port_by_id(node, 0); + port = of_graph_get_port_by_id(node, port_id); if (!port) return ERR_PTR(-EINVAL); @@ -826,8 +828,20 @@ sun4i_tcon_find_engine_traverse(struct sun4i_drv *drv, if (remote == engine->node) goto out_put_remote; + /* + * According to device tree binding input ports have even id + * number and output ports have odd id. Since component with + * more than one input and one output (TCON TOP) exits, correct + * remote input id has to be calculated by subtracting 1 from + * remote output id. If this for some reason can't be done, 0 + * is used as input port id. + */ + port = of_graph_get_remote_port(ep); + if (!of_property_read_u32(port, "reg", ®) && reg > 0) + reg -= 1; + /* keep looking through upstream ports */ - engine = sun4i_tcon_find_engine_traverse(drv, remote); + engine = sun4i_tcon_find_engine_traverse(drv, remote, reg); out_put_remote: of_node_put(remote); @@ -950,7 +964,7 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, /* Fallback to old method by traversing input endpoints */ of_node_put(port); - return sun4i_tcon_find_engine_traverse(drv, node); + return sun4i_tcon_find_engine_traverse(drv, node, 0); } static int sun4i_tcon_bind(struct device *dev, struct device *master,
Current "old" method to find engine worked pretty well for DE2. However, it doesn't work when TCON TOP is between mixer (engine) and TCON. TCON TOP has multiple input ports, but current engine search algorithm expects only one. This can be fixed by first looking for output port id and selecting matching input by subtracting 1 for the next round. This work even if there is only one input and output. Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)