Hi Ohad and Michal
yes I can share my patch, remoteproc has evolved and my patch is not
aligned on latest version of remote proc (especially since remoteproc:
remove the single rpmsg vdev limitation).
Ohad, for alignment I can take the latest branch of kernel.org
(remoteproc) branch for-next?
/BR
Ludovic Barr?
On 05/22/2012 07:51 AM, Michal Simek wrote:
> Hi Ohad and Ludovic,
>
> 2012/5/21 Ohad Ben-Cohen <[email protected] <mailto:[email protected]>>
>
> Hi Michal,
>
> On Mon, May 21, 2012 at 4:02 PM, Michal Simek <[email protected]
> <mailto:[email protected]>> wrote:
> > I have looked at it and tested on latest and greatest and the
> problem is
> > still there.
>
> Ok, I see why this is happening.
>
> We're now allocating the vrings' DMA in ->find_vqs() just before we
> boot the remote processor, and we release it on ->del_vqs(). These are
> the virtio handlers for setting up and tearing down the vrings, so it
> makes sense to do so, but as a result, we still don't get the early
> carveout allocation you wanted.
>
> > I have looked at the code and path is
> > rproc_fw_config_virtio -> rproc_handliefirtion_rsc ->
> rproc_handle_vdev
>
> Please note that rproc_handle_vdev doesn't allocate any DMA at this
> point, and the allocation path is now quite different - it begins with
> virtio_rpmsg_bus probing and then calling into the ->find_vqs()
> handler.
>
>
> > For me it is necessary to ensure that dma_alloc_coherent in
> > rproc_alloc_vring is called after carveout allocation
> > just because dma_alloc_cohorent takes mermory from preallocated
> pool which
> > starts at 0x0.
>
> ok.
>
> > I also don't have
> > any other advance mechanism how to ensure mapping for the second
> arm core.
>
> Probably the best way to ensure this (without it breaking unexpectedly
> again as remoteproc evolves) is to have separate sub-devices for
> different kind of memories, which remoteproc could then look up using
> something like device_find_child().
>
> Ludovic is exploring this direction too, because STE also needs to
> bind specific memory regions with predefined purposes, without
> depending on the order of DMA allocations.
>
>
> It is good that someone else has similar requirements.
>
>
> I think Ludovic already has a preliminary patch which he may be able
> to share with you.
>
>
> Ludovic: Can you share your patch with us? I would like to try it.
>
>
> > Here is log which I need to get. Look at attached patch which
> exactly show
> > you what I need to do which I have tested.
>
> Yes, I see. Another temporary approach might be moving the vrings'
> allocations to the rproc_handle_rsc table (in the RSC_VDEV slot), but
> I'm not sure we want to merge this as well because this solution too
> is only hiding the real issue and not solving it permanently.
>
>
> Simple enabling RSC_VDEV in rproc_handle_rsc doesn't work.
>
> BTW: I am using kernel modules and there is no dependency for
> virtio_rpmsg_bus in remoteproc module
> in sense if you load remoteproc module you also need to load
> virtio_rpmsg_bus to get firmware to work.
>
> I think that will be good to create that dependency automatically
> because you need to load virtio_rpmsg_bus
> when you problem remoteproc module.
>
> ~ # lsmod
> ~ # modprobe zynq_remoteproc
> zynq_remoteproc: Unknown symbol rproc_register (err 0)
> zynq_remoteproc: Unknown symbol rproc_alloc (err 0)
> zynq_remoteproc: Unknown symbol rproc_vq_interrupt (err 0)
> zynq_remoteproc: Unknown symbol rproc_free (err 0)
> zynq_remoteproc: Unknown symbol rproc_unregister (err 0)
> CPU1: shutdown
> zynq_remoteproc 0.remoteproc-test: 0.remoteproc-test is available
> zynq_remoteproc 0.remoteproc-test: Note: remoteproc is still under
> development and considered experimental.
> zynq_remoteproc 0.remoteproc-test: THE BINARY FORMAT IS NOT YET
> FINALIZED, and backward compatibility isn't yet guaranteed.
> ~ # zynq_remoteproc 0.remoteproc-test: registered virtio0 (type 7)
>
> ~ # lsmod
> zynq_remoteproc 4346 0 - Live 0xbf01f000
> remoteproc 13831 1 zynq_remoteproc, Live 0xbf018000
> virtio 2840 1 remoteproc, Live 0xbf008000
> virtio_ring 4965 1 remoteproc, Live 0xbf013000
> ~ #
> ~ # modprobe virtio_rpmsg_bus
> zynq_remoteproc 0.remoteproc-test: powering up 0.remoteproc-test
> zynq_remoteproc 0.remoteproc-test: Booting fw image test, size 2351790
> zynq_remoteproc 0.remoteproc-test: remote processor 0.remoteproc-test is
> now up
> virtio_rpmsg_bus virtio0: rpmsg host is online
> virtio_rpmsg_bus virtio0: creating channel rpmsg-timer-statistic addr 0x50
> ~ #
> ~ # lsmod
> virtio_rpmsg_bus 7575 0 - Live 0xbf024000
> zynq_remoteproc 4346 1 - Live 0xbf01f000
> remoteproc 13831 1 zynq_remoteproc, Live 0xbf018000
> virtio 2840 2 virtio_rpmsg_bus,remoteproc, Live 0xbf008000
> virtio_ring 4965 2 virtio_rpmsg_bus,remoteproc, Live 0xbf013000
> ~ #
>
> Can you see the same behavior on omap?
>
> Thanks,
> Michal
>
>
>
>
>
>
>
>
Hi Ludovic,
On Tue, May 22, 2012 at 12:14 PM, frq09524 <[email protected]> wrote:
> Ohad, for alignment I can take the latest branch of kernel.org (remoteproc)
> branch for-next?
Sure, it's pretty much updated sans a few simple changes.
Thanks,
Ohad.
hi Ohad
In my previous patch, to find the correct subdevice that match with
physical memory, I used pa member of rproc_mem_entry.
but today in these 2 resources: fw_rsc_trace, fw_rsc_vdev_vring, pa
menber has been removed.
for fw_rsc_trace it's not a problem, because use rproc_da_to_va and
carveout feature.
but for fw_rsc_vdev_vring, I think use da member, like this.
dev = rproc_dma_find_dev(rproc, (phys_addr_t) vring->da);
va = dma_alloc_coherent(dev, size, &dma, GFP_KERNEL);
if (!va) {
dev_err(dev, "dma_alloc_coherent failed\n");
return -EINVAL;
}
are you OK ?
/BR
Ludovic Barr?
On 05/22/2012 11:22 AM, Ohad Ben-Cohen wrote:
> Hi Ludovic,
>
> On Tue, May 22, 2012 at 12:14 PM, frq09524<[email protected]> wrote:
>> Ohad, for alignment I can take the latest branch of kernel.org (remoteproc)
>> branch for-next?
>
> Sure, it's pretty much updated sans a few simple changes.
>
> Thanks,
> Ohad.
Hi?Ludovic,
On Tue, May 22, 2012 at 3:51 PM, frq09524 <[email protected]> wrote:
> In my previous patch, to find the correct subdevice that match with physical memory, I used pa member of rproc_mem_entry.
I'm not sure we want to find these subdevices using the physical
memory of the expected region.
Doing so means more work when setting up those numbers initially, and
may also break if/when those number changes.
What do you think about looking up those subdevices by names?
E.g. before allocating the vrings, we may look for a subdevice called
"vrings-mem". If one exists, we'll use it when invoking the DMA API.
If not, we'll just use the default rproc device.
This way we don't need to change the firmware if/when we change the
memory regions allocated for each purpose.
Thanks,
Ohad.