Re: [PATCH v2 11/15] accel/qda: Add PRIME DMA-BUF import support

On 18-08-2026 09:25, Dmitry Baryshkov wrote:
> On Mon, Aug 17, 2026 at 10:17:46AM +0530, Ekansh Gupta wrote:
>> Allow user-space to import DMA-BUF file descriptors from other
>> subsystems (GPU, camera, video) into the QDA driver via the standard
>> DRM PRIME interface.
>>
>> qda_prime.c
>>   Implements qda_gem_prime_import(), which is set as the driver's
>>   .gem_prime_import callback. On import it:
>>   1. Short-circuits self-import: if the dma_buf was exported by this
>>      device and is not itself an import, the existing GEM object is
>>      returned with an incremented reference count.
>>   2. Attaches to the dma_buf and maps it with DMA_BIDIRECTIONAL via
>>      dma_buf_map_attachment_unlocked(), obtaining an sg_table whose
>>      DMA addresses are IOMMU virtual addresses in the CB device's
>>      address space.
>>   3. Calls qda_memory_manager_alloc() to record the mapping and encode
>>      the SID in the upper bits of the DMA address, matching the
>>      convention used for natively allocated buffers.
>>
>>   qda_prime_fd_to_handle() wraps drm_gem_prime_fd_to_handle() under
>>   qdev->import_lock, storing the calling file_priv in
> 
> Why do you need a separate lock?
Will add details in commit message,
The lock serialises the `fd_to_handle → gem_prime_import` call so that
`current_import_file_priv` is valid for the duration of the import.
Without it, two concurrent imports could race and each see the other's
`file_priv`.
> 
>>   qdev->current_import_file_priv so that qda_gem_prime_import() can
>>   retrieve it (the .gem_prime_import callback does not receive
>>   file_priv directly, but the context bank to attach to is per-process).
>>
>> qda_gem.c
>>   qda_gem_free_object() is extended to handle the imported-buffer
>>   teardown path: unmap the sg_table, detach from the dma_buf, and
>>   release the dma_buf reference.
>>   qda_gem_mmap_obj() rejects mmap requests on imported objects, which
>>   must be mapped through the exporter instead.
>>
>> qda_memory_manager.c
>>   The DSP is given a single base address per buffer, so only buffers
>>   that are mapped as one contiguous range can be described to it.
> 
> Is there no IOMMU in front of the DSP? Can we remap the buffer to a
> linear are inside the DSP map?
The DSP firmware's buffer descriptor format carries a single
{base_address, size} pair per buffer and it does not support
scatter-gather or multi-entry page tables on its side. This is a
firmware interface constraint that we cannot change from the kernel. So
imported buffers must present as one contiguous IOVA range, and
non-contiguous ones are rejected.

Will add this information in commit message and during the check.>
>>   qda_memory_manager_map_imported() therefore walks the imported
>>   buffer's scatterlist and rejects any buffer whose entries are not
>>   contiguous; contiguous imports (for example from the CMA DMA-buf
>>   heap) are accepted. Whether an exporter or IOMMU coalesces a buffer
>>   into a single range is not guaranteed, so this is verified rather
>>   than assumed.
>>   qda_memory_manager_free() skips the DMA free path for imported
>>   buffers since the memory is owned by the exporter.
>>
>> Assisted-by: Claude:claude-sonnet-5
>> Signed-off-by: Ekansh Gupta <[email protected]>
>> ---
>> Changes in v2:
>> - Do not assume the exporter or IOMMU maps an imported buffer as a
>>   single contiguous range; walk the scatterlist and cleanly reject
>>   non-contiguous imports instead (Christian König)
>> - Document that only contiguous imports (e.g. CMA DMA-buf heap) are
>>   supported
>> - Return -EBUSY rather than -ENODEV when no context bank is available
>> ---
>>  drivers/accel/qda/Makefile             |   1 +
>>  }
>>  
>> +static int qda_memory_manager_map_imported(struct qda_gem_obj *gem_obj,
>> +					   struct qda_iommu_device *iommu_dev)
>> +{
>> +	dma_addr_t dma_addr, expected_addr = 0;
>> +	struct scatterlist *sg, *s;
>> +	int i;
>> +
>> +	sg = gem_obj->sgt->sgl;
>> +
>> +	/*
>> +	 * Verify that the imported DMA-BUF is contiguous. While some platforms
> 
> Which "some platforms"?
I'll rewrite the comment to be precise.>
>> +	 * or IOMMUs might map buffers as a single range, we must not silently
>> +	 * assume this is always the case. We loop through the scatterlist
>> +	 * entries and reject any that are non-contiguous.
>> +	 */
>