On Fri, Jul 26, 2019 at 8:59 PM Jungo Lin [off-list ref] wrote:

Hi Robin:

On Fri, 2019-07-26 at 12:04 +0100, Robin Murphy wrote:

quoted

On 26/07/2019 08:42, Tomasz Figa wrote:

quoted

On Fri, Jul 26, 2019 at 4:41 PM Christoph Hellwig [off-list ref] wrote:

quoted

On Fri, Jul 26, 2019 at 02:15:14PM +0900, Tomasz Figa wrote:

quoted

Could you try dma_get_sgtable() with the SCP struct device and then
dma_map_sg() with the P1 struct device?

Please don't do that.  dma_get_sgtable is a pretty broken API (see
the common near the arm implementation) and we should not add more
users of it.  If you want a piece of memory that can be mapped to
multiple devices allocate it using alloc_pages and then just map
it to each device.

Thanks for taking a look at this thread.

Unfortunately that wouldn't work. We have a specific reserved memory
pool that is the only memory area accessible to one of the devices.
Any idea how to handle this?

If it's reserved in the sense of being outside struct-page-backed
"kernel memory", then provided you have a consistent CPU physical
address it might be reasonable for other devices to access it via
dma_map_resource().

Robin.

Thank you for your suggestion.

After revising to use dma_map_resource(), it is worked. Below is the
current implementation. Pleas kindly help us to check if there is any
misunderstanding.

#define MTK_ISP_COMPOSER_MEM_SIZE               0x200000

        /*
         * Allocate coherent reserved memory for SCP firmware usage.
         * The size of SCP composer's memory is fixed to 0x200000
         * for the requirement of firmware.
         */
        ptr = dma_alloc_coherent(p1_dev->cam_dev.smem_dev,
                                 MTK_ISP_COMPOSER_MEM_SIZE, &addr, GFP_KERNEL);
        if (!ptr) {
                dev_err(dev, "failed to allocate compose memory\n");
                return -ENOMEM;
        }
        p1_dev->composer_scp_addr = addr;
        p1_dev->composer_virt_addr = ptr;
        dev_dbg(dev, "scp addr:%pad va:%pK\n", &addr, ptr);

        /*
         * This reserved memory is also be used by ISP P1 HW.
         * Need to get iova address for ISP P1 DMA.
         */
        addr = dma_map_resource(dev, addr, MTK_ISP_COMPOSER_MEM_SIZE,
                                DMA_BIDIRECTIONAL, DMA_ATTR_SKIP_CPU_SYNC);

This is still incorrect, because addr is a DMA address, but the second
argument to dma_map_resource() is a physical address.

        if (dma_mapping_error(dev, addr)) {
                dev_err(dev, "Failed to map scp iova\n");
                ret = -ENOMEM;
                goto fail_free_mem;
        }
        p1_dev->composer_iova = addr;
        dev_info(dev, "scp iova addr:%pad\n", &addr);

Moreover, appropriate Tomasz & Christoph's help on this issue.

Robin, the memory is specified using the reserved-memory DT binding
and managed by the coherent DMA pool framework. We can allocate from
it using dma_alloc_coherent(), which gives us a DMA address, not CPU
physial address (although in practice on this platform they are equal
numerically).

Best regards,
Tomasz