On Wed 30 Nov 01:22 PST 2016, Stanimir Varbanov wrote: [..] > > Then I'd propose to add compatible string for msm8916 too. We will need > to distinguish between SoCs in order to select proper Venus firmware > file too. > This does make sense. > I think using power domain in remoteproc driver will break power > management of the v4l2 venus driver. Presently I use runtime pm to > switch ON/OFF GDSC and enable/disable Venus clocks. > > When the .probe of venus remoteproc driver is called the power domain > will become ON and never switched OFF (except when platform driver > .remove method is called). One possible solution would be to add runtime > pm in venus remoreproc driver. > > IMO we should think more about that. > > Bjorn, Stephen what are your opinions? > Sorry for not getting back to you on this after our earlier discussion on the topic. Modelling the remoteproc and codec drivers as completely separate entities (with only the rproc phandle) gives us issues with the timing between the two. As I pulled down and started playing with [1] I noticed that I was not able to get the codec driver to probe unless I compiled it as a module which I insmoded after my /lib/firmware became available. In addition to this, there's no way for the remoteproc driver to inform the codec driver when it's actually shutting down and booting back up during a crash recovery (an async operation). And lastly when I talked to Rob about the DT binding the obvious question of why one piece of hardware have two disjunct nodes in the DT. So I believe we should represent the codec driver as a child of the remoteproc driver, utilizing the newly introduced "remoteproc subdevices" to probe and remove the codec driver based on the state of the remoteproc driver. This relationship will also allow us to tie certain resources (e.g. the clocks and power-domain) to the remoteproc driver and use pm_runtime in either driver to make sure the resources are enabled appropriately. I did backport some patches from my v4.10 remoteproc pull request into [2] and merged this into [1] and made a prototype of this. You can find it here [3], I did test that I can boot the remoteproc and run v4l2-compliance, shut it down, boot it back up and re-run v4l2-compliance, and it seems to work fine. With these patches you can compile the remoteproc driver builtin to the kernel and once your system is booted you can issue: echo start > /sys/class/remoteproc/remoteproc0/state and venus should boot and the codec should be probed. echo stop > /sys/class/remoteproc/remoteproc0/state will shut down the core after removing the codec. Please let me know if you think this is a viable solution. (This is only tested on db410c so far, looks like we might need a few more clocks for 8996 and we should probably be explicit about the noc and its resources) [1] https://git.linaro.org/people/stanimir.varbanov/linux.git/log/?h=release/qcomlt-4.9-venus-wip [2] https://github.com/andersson/kernel/commits/qcomlt/v4.9-rproc-backport [3] https://github.com/andersson/kernel/commits/qcomlt/venus-wip Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
