Re: [PATCH 6/9] drm/tegra: gem: Add a clarifying comment

Thierry Reding
2021-03-24T15:02:30Z

[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]

On Wed, Mar 24, 2021 at 05:41:08PM +0300, Dmitry Osipenko wrote:
> 23.03.2021 18:54, Thierry Reding пишет:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Clarify when a fixed IOV address can be used and when a buffer has to
> > be mapped before the IOVA can be used.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > drivers/gpu/drm/tegra/plane.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
> > index 19e8847a164b..793da5d675d2 100644
> > --- a/drivers/gpu/drm/tegra/plane.c
> > +++ b/drivers/gpu/drm/tegra/plane.c
> > @@ -119,6 +119,14 @@ static int tegra_dc_pin(struct tegra_dc *dc, struct tegra_plane_state *state)
> > dma_addr_t phys_addr, *phys;
> > struct sg_table *sgt;
> >
> > + /*
> > + * If we're not attached to a domain, we already stored the
> > + * physical address when the buffer was allocated. If we're
> > + * part of a group that's shared between all display
> > + * controllers, we've also already mapped the framebuffer
> > + * through the SMMU. In both cases we can short-circuit the
> > + * code below and retrieve the stored IOV address.
> > + */
> > if (!domain || dc->client.group)
> > phys = &phys_addr;
> > else
> >
>
> This comment is correct, but the logic feels a bit lame because it
> should be wasteful to re-map DMA on each FB flip. Personally I don't
> care much about this since older Tegras use pinned buffers by default,
> but this shouldn't be good for T124+ users.

I'm not terribly thrilled by this either, but it's the only way to do
this when using the DMA API because we don't know at allocation time (or
import time for that matter) which of the (up to) 4 display controllers
a framebuffer will be shown on. tegra_dc_pin() is the earliest where
this is known and worst case that's called once per flip.

When the IOMMU API is used explicitly, we always map framebuffers into
the IOMMU domain shared by all display controllers at allocation or
import time and then we don't need to pin at flip time anymore.

I do have a work-in-progress patch somewhere that creates a mapping
cache to mitigate this problem to some degree. I need to dig that up and
do a few measurements because I vaguely recall this speeding up flips by
quite a bit (well, except for the very first mapping, obviously).

> Perhaps dumb buffers should be pinned to display by default and then we
> should extend the Tegra UAPI to support BO mapping to display client(?).

That would kind of defeat the purpose of a generic KMS UAPI.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]