From: Javier Martinez Canillas <javier@osg.samsung.com>
To: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Shuah Khan <shuahkh@osg.samsung.com>,
Sakari Ailus <sakari.ailus@iki.fi>
Subject: Re: [PATCH 2/2] [media] omap3isp: separate links creation from entities init
Date: Thu, 27 Aug 2015 09:59:59 +0200 [thread overview]
Message-ID: <55DEC37F.4080209@osg.samsung.com> (raw)
In-Reply-To: <20150826174337.32851e36@recife.lan>
Hello Mauro,
On 08/26/2015 10:43 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 26 Aug 2015 17:25:19 +0200
> Javier Martinez Canillas <javier@osg.samsung.com> escreveu:
>
>> The omap3isp driver initializes the entities and creates the pads links
>> before the entities are registered with the media device. This does not
>> work now that object IDs are used to create links so the media_device
>> has to be set.
>>
>> Split out the pads links creation from the entity initialization so are
>> made after the entities registration.
>>
>> Suggested-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>
> Did some tests there on a Beagleboard.
>
> That's what media-ctl reports before the patches:
>
> digraph board {
> rankdir=TB
> n00000001 [label="{{<port0> 0} | OMAP3 ISP CCP2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000001:port1 -> n00000005:port0 [style=dashed]
> n00000002 [label="OMAP3 ISP CCP2 input\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> n00000002 -> n00000001:port0 [style=dashed]
> n00000003 [label="{{<port0> 0} | OMAP3 ISP CSI2a | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000003:port1 -> n00000004 [style=dashed]
> n00000003:port1 -> n00000005:port0 [style=dashed]
> n00000004 [label="OMAP3 ISP CSI2a output\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> n00000005 [label="{{<port0> 0} | OMAP3 ISP CCDC | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000005:port1 -> n00000006 [style=dashed]
> n00000005:port2 -> n00000007:port0 [style=dashed]
> n00000005:port1 -> n0000000a:port0 [style=dashed]
> n00000005:port2 -> n0000000d:port0 [style=bold]
> n00000005:port2 -> n0000000e:port0 [style=bold]
> n00000005:port2 -> n0000000f:port0 [style=bold]
> n00000006 [label="OMAP3 ISP CCDC output\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> n00000007 [label="{{<port0> 0} | OMAP3 ISP preview | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000007:port1 -> n00000009 [style=dashed]
> n00000007:port1 -> n0000000a:port0 [style=dashed]
> n00000008 [label="OMAP3 ISP preview input\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
> n00000008 -> n00000007:port0 [style=dashed]
> n00000009 [label="OMAP3 ISP preview output\n/dev/video4", shape=box, style=filled, fillcolor=yellow]
> n0000000a [label="{{<port0> 0} | OMAP3 ISP resizer | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n0000000a:port1 -> n0000000c [style=dashed]
> n0000000b [label="OMAP3 ISP resizer input\n/dev/video5", shape=box, style=filled, fillcolor=yellow]
> n0000000b -> n0000000a:port0 [style=dashed]
> n0000000c [label="OMAP3 ISP resizer output\n/dev/video6", shape=box, style=filled, fillcolor=yellow]
> n0000000d [label="{{<port0> 0} | OMAP3 ISP AEWB | {}}", shape=Mrecord, style=filled, fillcolor=green]
> n0000000e [label="{{<port0> 0} | OMAP3 ISP AF | {}}", shape=Mrecord, style=filled, fillcolor=green]
> n0000000f [label="{{<port0> 0} | OMAP3 ISP histogram | {}}", shape=Mrecord, style=filled, fillcolor=green]
> }
>
> And those are what's reported after the changes:
>
> digraph board {
> rankdir=TB
> n00000001 [label="{{<port0> 0} | OMAP3 ISP CCP2 | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000001:port1 -> n00000005:port0 [style=dashed]
> n00000002 [label="OMAP3 ISP CCP2 input\n/dev/video0", shape=box, style=filled, fillcolor=yellow]
> n00000002 -> n00000001:port0 [style=dashed]
> n00000003 [label="{{<port0> 0} | OMAP3 ISP CSI2a | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000003:port1 -> n00000004 [style=dashed]
> n00000003:port1 -> n00000005:port0 [style=dashed]
> n00000004 [label="OMAP3 ISP CSI2a output\n/dev/video1", shape=box, style=filled, fillcolor=yellow]
> n00000005 [label="{{<port0> 0} | OMAP3 ISP CCDC | {<port1> 1 | <port2> 2}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000005:port1 -> n00000006 [style=dashed]
> n00000005:port2 -> n00000007:port0 [style=dashed]
> n00000005:port1 -> n0000000a:port0 [style=dashed]
> n00000005:port2 -> n0000000d:port0 [style=bold]
> n00000005:port2 -> n0000000e:port0 [style=bold]
> n00000005:port2 -> n0000000f:port0 [style=bold]
> n00000006 [label="OMAP3 ISP CCDC output\n/dev/video2", shape=box, style=filled, fillcolor=yellow]
> n00000007 [label="{{<port0> 0} | OMAP3 ISP preview | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n00000007:port1 -> n00000009 [style=dashed]
> n00000007:port1 -> n0000000a:port0 [style=dashed]
> n00000008 [label="OMAP3 ISP preview input\n/dev/video3", shape=box, style=filled, fillcolor=yellow]
> n00000008 -> n00000007:port0 [style=dashed]
> n00000009 [label="OMAP3 ISP preview output\n/dev/video4", shape=box, style=filled, fillcolor=yellow]
> n0000000a [label="{{<port0> 0} | OMAP3 ISP resizer | {<port1> 1}}", shape=Mrecord, style=filled, fillcolor=green]
> n0000000a:port1 -> n0000000c [style=dashed]
> n0000000b [label="OMAP3 ISP resizer input\n/dev/video5", shape=box, style=filled, fillcolor=yellow]
> n0000000b -> n0000000a:port0 [style=dashed]
> n0000000c [label="OMAP3 ISP resizer output\n/dev/video6", shape=box, style=filled, fillcolor=yellow]
> n0000000d [label="{{<port0> 0} | OMAP3 ISP AEWB | {}}", shape=Mrecord, style=filled, fillcolor=green]
> n0000000e [label="{{<port0> 0} | OMAP3 ISP AF | {}}", shape=Mrecord, style=filled, fillcolor=green]
> n0000000f [label="{{<port0> 0} | OMAP3 ISP histogram | {}}", shape=Mrecord, style=filled, fillcolor=green]
> }
>
>
> With is exactly the same graph.
>
> I also ran the my G_TOPOLOGY tool. Of course, it fails before the
> patches, working properly after them.
>
> After the patches, it reports entities, links, pads, interfaces and
> interface links as it should be:
>
> $ mc_nextgen_test -e -i -I -l
> version: 80number of entities: 15
> number of interfaces: 7
> number of pads: 21
> number of links: 37
> entity entity#1: OMAP3 ISP CCP2, 2 pad(s), 1 source(s)
> entity entity#2: OMAP3 ISP CCP2 input, 1 pad(s)
> entity entity#3: OMAP3 ISP CSI2a, 2 pad(s), 1 source(s)
> entity entity#4: OMAP3 ISP CSI2a output, 1 pad(s)
> entity entity#5: OMAP3 ISP CCDC, 3 pad(s), 2 source(s)
> entity entity#6: OMAP3 ISP CCDC output, 1 pad(s)
> entity entity#7: OMAP3 ISP preview, 2 pad(s), 1 source(s)
> entity entity#8: OMAP3 ISP preview input, 1 pad(s)
> entity entity#9: OMAP3 ISP preview output, 1 pad(s)
> entity entity#10: OMAP3 ISP resizer, 2 pad(s), 1 source(s)
> entity entity#11: OMAP3 ISP resizer input, 1 pad(s)
> entity entity#12: OMAP3 ISP resizer output, 1 pad(s)
> entity entity#13: OMAP3 ISP AEWB, 1 pad(s)
> entity entity#14: OMAP3 ISP AF, 1 pad(s)
> entity entity#15: OMAP3 ISP histogram, 1 pad(s)
> interface intf_devnode#1: video (81,0)
> interface intf_devnode#2: video (81,1)
> interface intf_devnode#3: video (81,2)
> interface intf_devnode#4: video (81,3)
> interface intf_devnode#5: video (81,4)
> interface intf_devnode#6: video (81,5)
> interface intf_devnode#7: video (81,6)
> interface link link#1: intf_devnode#1 <=> entity#2
> interface link link#2: intf_devnode#2 <=> entity#4
> interface link link#3: intf_devnode#3 <=> entity#6
> interface link link#4: intf_devnode#4 <=> entity#8
> interface link link#5: intf_devnode#5 <=> entity#9
> interface link link#6: intf_devnode#6 <=> entity#11
> interface link link#7: intf_devnode#7 <=> entity#12
> data link link#8: pad#5 => pad#6
> data link link#9: pad#5 => pad#6
> data link link#10: pad#3 => pad#1
> data link link#11: pad#3 => pad#1
> data link link#12: pad#8 => pad#10
> data link link#13: pad#8 => pad#10
> data link link#14: pad#13 => pad#11
> data link link#15: pad#13 => pad#11
> data link link#16: pad#12 => pad#14
> data link link#17: pad#12 => pad#14
> data link link#18: pad#17 => pad#15
> data link link#19: pad#17 => pad#15
> data link link#20: pad#16 => pad#18
> data link link#21: pad#16 => pad#18
> data link link#22: pad#5 => pad#7
> data link link#23: pad#5 => pad#7
> data link link#24: pad#2 => pad#7
> data link link#25: pad#2 => pad#7
> data link link#26: pad#9 => pad#11
> data link link#27: pad#9 => pad#11
> data link link#28: pad#8 => pad#15
> data link link#29: pad#8 => pad#15
> data link link#30: pad#12 => pad#15
> data link link#31: pad#12 => pad#15
> data link link#32: pad#9 => pad#19 [IMMUTABLE] [ENABLED]
> data link link#33: pad#9 => pad#19 [IMMUTABLE] [ENABLED]
> data link link#34: pad#9 => pad#20 [IMMUTABLE] [ENABLED]
> data link link#35: pad#9 => pad#20 [IMMUTABLE] [ENABLED]
> data link link#36: pad#9 => pad#21 [IMMUTABLE] [ENABLED]
> data link link#37: pad#9 => pad#21 [IMMUTABLE] [ENABLED]
>
> Everything is working as it should.
>
> So:
>
> Tested-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
Thanks a lot for testing. I'll audit other media platform drivers to see
if they need a similar patch and try to address those if that's the case.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
prev parent reply other threads:[~2015-08-27 8:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-26 15:25 [PATCH 0/2] Patches to test MC next gen patches in OMAP3 ISP Javier Martinez Canillas
2015-08-26 15:25 ` [PATCH 1/2] [media] media: don't try to empty links list in media_entity_cleanup() Javier Martinez Canillas
2015-11-23 15:42 ` Laurent Pinchart
2015-08-26 15:25 ` [PATCH 2/2] [media] omap3isp: separate links creation from entities init Javier Martinez Canillas
2015-08-26 20:43 ` Mauro Carvalho Chehab
2015-08-27 7:59 ` Javier Martinez Canillas [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55DEC37F.4080209@osg.samsung.com \
--to=javier@osg.samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@osg.samsung.com \
--cc=sakari.ailus@iki.fi \
--cc=shuahkh@osg.samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.