Re: [PATCH v3 19/20] drm/tegra: Implement new UAPI

From: Mikko Perttunen <cyndis@kapsi.fi>
To: Daniel Vetter <daniel@ffwll.ch>, Dmitry Osipenko <digetx@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jon Hunter <jonathanh@nvidia.com>, Dave Airlie <airlied@linux.ie>,
	linux-tegra <linux-tegra@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	talho@nvidia.com, bhuntsman@nvidia.com
Subject: Re: [PATCH v3 19/20] drm/tegra: Implement new UAPI
Date: Tue, 20 Oct 2020 15:51:01 +0300	[thread overview]
Message-ID: <ec4138c0-6c7f-b32e-2049-7848b6ac7f6b@kapsi.fi> (raw)
In-Reply-To: <CAKMK7uFWyMZQauakjzSWa9r494R4JKDkAk6ABZOLLsCXb6_yHg@mail.gmail.com>

On 10/20/20 2:40 PM, Daniel Vetter wrote:
> On Mon, Oct 19, 2020 at 7:27 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 19.10.2020 11:13, Mikko Perttunen пишет:
>>> On 10/19/20 5:21 AM, Dmitry Osipenko wrote:
>>>> 07.10.2020 20:12, Mikko Perttunen пишет:
>>>>> +int tegra_drm_ioctl_channel_map(struct drm_device *drm, void *data,
>>>>> +                struct drm_file *file)
>>>>> +{
>>>>
>>>> Hello, Mikko!
>>>>
>>>> Could you please tell what are the host1x clients that are going to be
>>>> upstreamed and will need this IOCTL?
>>>>
>>>
>>> Hi Dmitry!
>>>
>>> It is needed for any engine/job that wants to access memory, as this
>>> IOCTL must be used to map memory for the engine. So all of them.
>>>
>>> Downstream doesn't have an equivalent IOCTL because it (currently) does
>>> mapping at submit time, but that is suboptimal because
>>>
>>> - it requires doing relocations in the kernel which isn't required for
>>> T186+
>>> - it's a big performance penalty, due to which the downstream kernel has
>>> the "deferred dma-buf unmapping" feature, where unmapping a dma_buf may
>>> not immediately unmap it in case it's used later, so that the "mapping"
>>> later is faster. A feature which we'd preferably get rid of.
>>> - because of the above feature not being controlled by the user, it can
>>> cause variance in submit times.
>>>
>>> On the other hand, we cannot (at least always) do the mapping at
>>> allocation/import time, because
>>>
>>> - A single FD may have multiple channel_ctx's, and an allocation/import
>>> may need to be used in any subset of them
>>> - The import IOCTL is fixed and doesn't have the parameters we'd need to
>>> do this at import time
>>> - Overall it's more orthogonal to have GEM object acquirement in one
>>> step and mapping in another.
>>>
>>> Maybe that's not quite what you asked, but it's some background anyway :)
>>
>> I'm asking this question because right now there is only one potential
>> client for this IOCTL, the VIC. If other clients aren't supposed to be a
>> part of the DRM driver, like for example NVDEC which probably should be
>> a V4L driver, then DRM driver will have only a single VIC and in this
>> case we shouldn't need this IOCTL because DRM and V4L should use generic
>> dmabuf API for importing and exporting buffers.
> 
> Randomly jumping in here ...
> 
> So if you have a drm driver with userspace in mesa3d already, the
> usual approach is to have a libva implementation (ideally in mesa3d
> too, using the gallium framework so that a lot of the boring
> integration glue is taken care of already) directly on top of drm. No
> v4l driver needed at all here.
> 
> And it sounds like this nvdec thing would fit that bill pretty neatly.
Something like this would be my preference as well.

Mikko

> 
>> I'm also not quite sure about whether the current model of the unified
>> Tegra DRM driver is suitable for having the separated engines. Perhaps
>> each separated engine should just have its own rendering node?
> 
> Above model using libva driver in userspace for nvdec would avoid this
> issue too.
> -Daniel
>