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

[Dmitry Osipenko <digetx@gmail.com>]

09.11.2020 17:53, Mikko Perttunen пишет:
...
> I guess for the channel_map we can drop the offset/length, I just think
> it's fairly obvious that an IOMMU mapping API lets you specify from
> where and how much you want to map. Sure, it's not a functionality
> blocker as it can simply be implemented in userspace by shifting the
> reloc offset / IOVA equivalently, but it will reduce IO address space
> usage and prevent access to memory that was not intended to be mapped to
> the engine.

It could be a good feature, but I'd want to see how userspace will
benefit from using it in practice before putting effort into it.

Could you please give examples of how this feature will be useful for
userspace? What is the use-case for allocating a single buffer and then
mapping it partially? Is this needed for a userspace memory pool or
something else?

...
> I am well aware of that. I'm not saying that we should copy the
> downstream stack. I am saying that when designing an ABI, we should
> consider all information available on what kind of features would be
> required from it.
> 
> Going through the proposed TegraDRM UAPI, there are some features that
> would probably not be immediately used by userspace, or supported in a
> non-NOOP fashion by the kernel:
> * Map offset/length
> * IOVA of mapping
> * Creation of sync_file postfence
> * Waiting for sync_file prefences
> * SUBMIT_CONTEXT_SETUP flag
> * Having two syncpt_incrs
> * Reservations?
> 
> I suppose we can remove all of that for now, as long as we ensure that
> there is a path to add them back. I'm just a bit concerned that we'll
> end up with 10 different ABI revisions and userspace will have to do a
> version detection dance and enable things depending on driver version.
> 
> Anything else to remove?

I guess it should be enough for now.

Reservations are needed for the grate drivers, but they need to be done
in conjunction with the DRM scheduler. I guess it should be fine if
you'll remove reservations, I'll then take a look at how to integrate
them and drm-sched on top of yours changes.

> Regarding things like explicit channel_map, sure, we could map things
> implicitly at submit time, but it is a huge performance improvement on
> newer chips, at least. So technically userspace doesn't need it, but
> going by that argument, we can get rid of a lot of kernel functionality
> -- after all, it's only needed if you want your hardware to perform well.

I have no doubt that it's better to have static mappings, but it's not
obvious how it should be implemented without seeing a full picture. I
mean that maybe it could be possible to avoid having these special
IOCTLs by changing the way of how hardware is exposed to userspace such
that generic UAPI could be used in order to achieve the same goals.

...
> If it is known to deadlock, we should fix it. In any case, which kind of
> scheduler is used shouldn't affect the ABI, and we already have a
> functional implemention in the Host1x driver, so we should merge the ABI
> first rather than wait for another year while the rest of the driver is
> redesigned and rewritten.

Perhaps should be fine to start extending the ABI, but then it should
stay under DRM_TEGRA_STAGING until we will be confident that it's all good.