[PATCH v2 2/5] drm/virtio: Add a helper to map and note the dma addrs and lengths

[Dmitry Osipenko]

On 10/24/24 08:49, Kasireddy, Vivek wrote:
> Hi Dmitry,
> 
>> Subject: Re: [PATCH v2 2/5] drm/virtio: Add a helper to map and note the
>> dma addrs and lengths
>>
>> On 10/22/24 07:51, Kasireddy, Vivek wrote:
>>> Hi Dmitry,
>>>
>>>>
>>>> On 8/13/24 06:49, Vivek Kasireddy wrote:
>>>>> +long virtgpu_dma_buf_import_sgt(struct virtio_gpu_mem_entry
>> **ents,
>>>>> +				unsigned int *nents,
>>>>> +				struct virtio_gpu_object *bo,
>>>>> +				struct dma_buf_attachment *attach)
>>>>> +{
>>>>> +	struct scatterlist *sl;
>>>>> +	struct sg_table *sgt;
>>>>> +	long i, ret;
>>>>> +
>>>>> +	dma_resv_assert_held(attach->dmabuf->resv);
>>>>> +
>>>>> +	ret = dma_resv_wait_timeout(attach->dmabuf->resv,
>>>>> +				    DMA_RESV_USAGE_KERNEL,
>>>>> +				    false, MAX_SCHEDULE_TIMEOUT);
>>>>
>>>> Why this wait is needed?
>>> The intention was to wait for any pending operations on the backing object
>>> to finish and let it become idle before mapping and accessing the
>> underlying
>>> memory. But I suspect this wait may not be necessary given that we would
>>> have already called dma_buf_pin() at this point, which would have had the
>>> desired effect?
>>
>> Looking at the dma_buf_map_attachment() code, I see that it does both of
>> pinning and waiting for the fence by itself. Hence should be safe to
>> drop both dma_buf_pin() and dma_resv_wait_timeout(), please test.
> Sure, I'll retest again but it looks like dma_buf_map_attachment() pins and
> or waits for fence only in specific situations. That is, it pins only if the exporter
> is non-dynamic and if CONFIG_DMABUF_MOVE_NOTIFY is not enabled. And,
> it waits for the fence only if the importer is non-dynamic. Since I have only tested
> with a dynamic exporter (Xe driver), I did not encounter any issues but it makes
> sense to augment the code to account for non-dynamic exporters as well.

If exporter or importer is not dynamic, then dma-buf core pins buffer at
the buffer importing time, see dma_buf_attach(). Hence, I expect that
everything should work fine.

>> BTW, is any DG2 GPU suitable for testing of this patchset? Will I be
>> able to test it using a regular consumer A750 card?
> Yes, you can test with any DG2 dGPU as long as you can passthrough it to the
> Guest VM. And, if there is an iGPU available on the Host, you can use GTK/SDL UI
> for local display or Spice UI for remote display if there is no iGPU on the Host.
> This is exactly how I started testing this patch series but I am now predominantly
> testing this series with SRIOV enabled iGPUs and dGPUs.

Was hoping to try out SR-IOV on A750 if it's even possible at all. But
passthrough indeed is another an option, will try with passthrough, thanks.

-- 
Best regards,
Dmitry