[PATCH v5 15/21] drm/tegra: Add new UAPI to header

[Dmitry Osipenko]

23.03.2021 15:30, Thierry Reding пишет:
> On Thu, Jan 14, 2021 at 12:34:22PM +0200, Mikko Perttunen wrote:
>> On 1/14/21 10:36 AM, Dmitry Osipenko wrote:
>>> 13.01.2021 21:56, Mikko Perttunen пишет:
>>>> On 1/13/21 8:14 PM, Dmitry Osipenko wrote:
>>>>> 11.01.2021 16:00, Mikko Perttunen пишет:
>>>>>> +struct drm_tegra_submit_buf {
>>>>>> +    /**
>>>>>> +     * @mapping_id: [in]
>>>>>> +     *
>>>>>> +     * Identifier of the mapping to use in the submission.
>>>>>> +     */
>>>>>> +    __u32 mapping_id;
>>>>>
>>>>> I'm now in process of trying out the UAPI using grate drivers and this
>>>>> becomes the first obstacle.
>>>>>
>>>>> Looks like this is not going to work well for older Tegra SoCs, in
>>>>> particular for T20, which has a small GART.
>>>>>
>>>>> Given that the usefulness of the partial mapping feature is very
>>>>> questionable until it will be proven with a real userspace, we should
>>>>> start with a dynamic mappings that are done at a time of job submission.
>>>>>
>>>>> DRM already should have everything necessary for creating and managing
>>>>> caches of mappings, grate kernel driver has been using drm_mm_scan for a
>>>>> long time now for that.
>>>>>
>>>>> It should be fine to support the static mapping feature, but it should
>>>>> be done separately with the drm_mm integration, IMO.
>>>>>
>>>>> What do think?
>>>>>
>>>>
>>>> Can you elaborate on the requirements to be able to use GART? Are there
>>>> any other reasons this would not work on older chips?
>>>
>>> We have all DRM devices in a single address space on T30+, hence having
>>> duplicated mappings for each device should be a bit wasteful.
>>
>> I guess this should be pretty easy to change to only keep one mapping per
>> GEM object.
> 
> The important point here is the semantics: this IOCTL establishes a
> mapping for a given GEM object on a given channel. If the underlying
> implementation is such that the mapping doesn't fit into the GART, then
> that's an implementation detail that the driver needs to take care of.
> Similarly, if multiple devices share a single address space, that's
> something the driver already knows and can take advantage of by simply
> reusing an existing mapping if one already exists. In both cases the
> semantics would be correctly implemented and that's really all that
> matters.
> 
> Overall this interface seems sound from a high-level point of view and
> allows these mappings to be properly created even for the cases we have
> where each channel may have a separate address space. It may not be the
> optimal interface for all use-cases or any one individual case, but the
> very nature of these interfaces is to abstract away certain differences
> in order to provide a unified interface to a common programming model.
> So there will always be certain tradeoffs.

For now this IOCTL isn't useful from a userspace perspective of older
SoCs and I'll need to add a lot of code that won't do anything useful
just to conform to the specific needs of the newer SoCs. Trying to unify
everything into a single API doesn't sound like a good idea at this
point and I already suggested to Mikko to try out variant with a
separated per-SoC code paths in the next version, then the mappings
could be handled separately by the T186+ paths.