Re: [PATCH 1/2] gpu: host1x: Allow entries in BO caches to be freed

[Mikko Perttunen]

On Monday, May 18, 2026 5:02 AM Aaron Kling wrote:
> On Thu, May 14, 2026 at 9:35 PM Mikko Perttunen <mperttunen@xxxxxxxxxx> wrote:
> >
> > When a buffer object is pinned via host1x_bo_pin() with a cache, the
> > resulting mapping is kept in the cache so it can be reused on subsequent
> > pins. Each mapping held a reference to the underlying host1x_bo (taken
> > in tegra_bo_pin / gather_bo_pin), so as long as a mapping was cached,
> > the bo itself could not be freed.
> >
> > However, the only way to remove the cached mapping was through the free
> > path of the buffer object. This meant that if a bo got cached, it could
> > never get freed again.
> >
> > Resolve the circularity by holding a weak reference to the bo from the
> > cache side. This is done by having the .pin callbacks not bump the bo's
> > refcount -- instead the common Host1x bo code does so, except for the
> > cache reference.
> >
> > Also move the remove-cache-mapping-on-free code into a common function
> > inside Host1x code. This is only called from the TegraDRM GEM buffers
> > since those are the only ones that can be cached at the moment.
> >
> > Reported-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> > Fixes: 1f39b1dfa53c ("drm/tegra: Implement buffer object cache")
> > Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/tegra/gem.c    | 13 ++-------
> >  drivers/gpu/drm/tegra/submit.c |  3 +--
> >  drivers/gpu/host1x/bus.c       | 60 +++++++++++++++++++++++++++++++++++++++++-
> >  include/linux/host1x.h         |  7 +++++
> >  4 files changed, 69 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> > index d2bae88ad545..2377e2b76397 100644
> > --- a/drivers/gpu/drm/tegra/gem.c
> > +++ b/drivers/gpu/drm/tegra/gem.c
> > @@ -69,7 +69,7 @@ static struct host1x_bo_mapping *tegra_bo_pin(struct device *dev, struct host1x_
> >                 return ERR_PTR(-ENOMEM);
> >
> >         kref_init(&map->ref);
> > -       map->bo = host1x_bo_get(bo);
> > +       map->bo = bo;
> >         map->direction = direction;
> >         map->dev = dev;
> >
> > @@ -170,7 +170,6 @@ static void tegra_bo_unpin(struct host1x_bo_mapping *map)
> >                 kfree(map->sgt);
> >         }
> >
> > -       host1x_bo_put(map->bo);
> >         kfree(map);
> >  }
> >
> > @@ -509,17 +508,9 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
> >  void tegra_bo_free_object(struct drm_gem_object *gem)
> >  {
> >         struct tegra_drm *tegra = gem->dev->dev_private;
> > -       struct host1x_bo_mapping *mapping, *tmp;
> >         struct tegra_bo *bo = to_tegra_bo(gem);
> >
> > -       /* remove all mappings of this buffer object from any caches */
> > -       list_for_each_entry_safe(mapping, tmp, &bo->base.mappings, list) {
> > -               if (mapping->cache)
> > -                       host1x_bo_unpin(mapping);
> > -               else
> > -                       dev_err(gem->dev->dev, "mapping %p stale for device %s\n", mapping,
> > -                               dev_name(mapping->dev));
> > -       }
> > +       host1x_bo_clear_cached_mappings(&bo->base);
> >
> >         if (tegra->domain) {
> >                 tegra_bo_iommu_unmap(tegra, bo);
> > diff --git a/drivers/gpu/drm/tegra/submit.c b/drivers/gpu/drm/tegra/submit.c
> > index 3009b8b9e619..e5841857c937 100644
> > --- a/drivers/gpu/drm/tegra/submit.c
> > +++ b/drivers/gpu/drm/tegra/submit.c
> > @@ -76,7 +76,7 @@ gather_bo_pin(struct device *dev, struct host1x_bo *bo, enum dma_data_direction
> >                 return ERR_PTR(-ENOMEM);
> >
> >         kref_init(&map->ref);
> > -       map->bo = host1x_bo_get(bo);
> > +       map->bo = bo;
> >         map->direction = direction;
> >         map->dev = dev;
> >
> > @@ -117,7 +117,6 @@ static void gather_bo_unpin(struct host1x_bo_mapping *map)
> >         dma_unmap_sgtable(map->dev, map->sgt, map->direction, 0);
> >         sg_free_table(map->sgt);
> >         kfree(map->sgt);
> > -       host1x_bo_put(map->bo);
> >
> >         kfree(map);
> >  }
> > diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> > index f814eb4941c0..772e05a7b45b 100644
> > --- a/drivers/gpu/host1x/bus.c
> > +++ b/drivers/gpu/host1x/bus.c
> > @@ -887,6 +887,20 @@ int host1x_client_resume(struct host1x_client *client)
> >  }
> >  EXPORT_SYMBOL(host1x_client_resume);
> >
> > +/**
> > + * host1x_bo_pin() - Create a DMA mapping for the buffer object
> > + * @dev: Device onto which DMA map to
> > + * @bo: Buffer object to map
> > + * @dir: DMA direction
> > + * @cache: Cache in which to store mapping, or NULL
> > + *
> > + * Creates a DMA mapping pointing to @bo for @dev. The refcount of @bo is incremented
> > + * until host1x_bo_unpin is called.
> > + *
> > + * If @cache is specified, the mapping is also stored in the cache and not released
> > + * until @bo is freed (refcount drops to zero). This improves performance when a buffer
> > + * is pinned and unpinned frequently as in the case of display use.
> > + */
> >  struct host1x_bo_mapping *host1x_bo_pin(struct device *dev, struct host1x_bo *bo,
> >                                         enum dma_data_direction dir,
> >                                         struct host1x_bo_cache *cache)
> > @@ -899,6 +913,7 @@ struct host1x_bo_mapping *host1x_bo_pin(struct device *dev, struct host1x_bo *bo
> >                 list_for_each_entry(mapping, &cache->mappings, entry) {
> >                         if (mapping->bo == bo && mapping->direction == dir) {
> >                                 kref_get(&mapping->ref);
> > +                               host1x_bo_get(bo);
> >                                 goto unlock;
> >                         }
> >                 }
> > @@ -908,6 +923,8 @@ struct host1x_bo_mapping *host1x_bo_pin(struct device *dev, struct host1x_bo *bo
> >         if (IS_ERR(mapping))
> >                 goto unlock;
> >
> > +       host1x_bo_get(bo);
> > +
> >         spin_lock(&mapping->bo->lock);
> >         list_add_tail(&mapping->list, &bo->mappings);
> >         spin_unlock(&mapping->bo->lock);
> > @@ -918,7 +935,12 @@ struct host1x_bo_mapping *host1x_bo_pin(struct device *dev, struct host1x_bo *bo
> >
> >                 list_add_tail(&mapping->entry, &cache->mappings);
> >
> > -               /* bump reference count to track the copy in the cache */
> > +               /*
> > +                * Bump the mapping reference count to track the mapping in the cache,
> > +                * but do not bump the BO's refcount. This allows the BO to still get freed,
> > +                * triggering the release of the cache mapping through
> > +                * host1x_bo_clear_cached_mappings.
> > +                */
> >                 kref_get(&mapping->ref);
> >         }
> >
> > @@ -948,9 +970,17 @@ static void __host1x_bo_unpin(struct kref *ref)
> >         mapping->bo->ops->unpin(mapping);
> >  }
> >
> > +/**
> > + * host1x_bo_unpin() - Release an established DMA mapping of a buffer object
> > + * @mapping: Mapping to release
> > + *
> > + * Unmaps the given @mapping, unless it is cached. Decreases the refcount on
> > + * the underlying buffer object.
> > + */
> >  void host1x_bo_unpin(struct host1x_bo_mapping *mapping)
> >  {
> >         struct host1x_bo_cache *cache = mapping->cache;
> > +       struct host1x_bo *bo = mapping->bo;
> >
> >         if (cache)
> >                 mutex_lock(&cache->lock);
> > @@ -959,5 +989,33 @@ void host1x_bo_unpin(struct host1x_bo_mapping *mapping)
> >
> >         if (cache)
> >                 mutex_unlock(&cache->lock);
> > +
> > +       host1x_bo_put(bo);
> >  }
> >  EXPORT_SYMBOL(host1x_bo_unpin);
> > +
> > +/**
> > + * host1x_bo_clear_cached_mappings() - Remove all cached mappings pointing at a bo
> > + * @bo: Buffer object to release mappings of
> > + *
> > + * Drops references to any mappings pointing to @bo left in any caches. This must
> > + * be called by any host1x_bo implementers that may be pinned with caching enabled
> > + * before freeing the bo.
> > + */
> > +void host1x_bo_clear_cached_mappings(struct host1x_bo *bo)
> > +{
> > +       struct host1x_bo_mapping *mapping, *tmp;
> > +       struct host1x_bo_cache *cache;
> > +
> > +       list_for_each_entry_safe(mapping, tmp, &bo->mappings, list) {
> > +               cache = mapping->cache;
> > +               if (WARN_ON(!cache))
> > +                       continue;
> > +
> > +               mutex_lock(&mapping->cache->lock);
> > +               WARN_ON(kref_read(&mapping->ref) != 1);
> > +               __host1x_bo_unpin(&mapping->ref);
> > +               mutex_unlock(&mapping->cache->lock);
> > +       }
> > +}
> > +EXPORT_SYMBOL(host1x_bo_clear_cached_mappings);
> > diff --git a/include/linux/host1x.h b/include/linux/host1x.h
> > index 5e7a63143a4a..d8f052a85b75 100644
> > --- a/include/linux/host1x.h
> > +++ b/include/linux/host1x.h
> > @@ -143,6 +143,12 @@ static inline struct host1x_bo_mapping *to_host1x_bo_mapping(struct kref *ref)
> >         return container_of(ref, struct host1x_bo_mapping, ref);
> >  }
> >
> > +/**
> > + * struct host1x_bo_ops - operations implemented by a host1x_bo provider
> > + *
> > + * @pin: create a DMA mapping. Implementation must not touch the bo's refcount.
> > + * @unpin: destroy a DMA mapping. Implementation must not touch the bo's refcount.
> > + */
> >  struct host1x_bo_ops {
> >         struct host1x_bo *(*get)(struct host1x_bo *bo);
> >         void (*put)(struct host1x_bo *bo);
> > @@ -181,6 +187,7 @@ struct host1x_bo_mapping *host1x_bo_pin(struct device *dev, struct host1x_bo *bo
> >                                         enum dma_data_direction dir,
> >                                         struct host1x_bo_cache *cache);
> >  void host1x_bo_unpin(struct host1x_bo_mapping *map);
> > +void host1x_bo_clear_cached_mappings(struct host1x_bo *bo);
> >
> >  static inline void *host1x_bo_mmap(struct host1x_bo *bo)
> >  {
> >
> > --
> > 2.53.0
> >
> 
> I have verified this on a Jetson AGX Xavier devkit, a Jetson Xavier NX
> devkit, a Jetson TX2 devkit, and a Jetson TX2 NX in a p3509 carrier.
> No longer seeing allocation failures nor am I seeing any obvious
> regressions. I am currently unable to boot any t210 device to Android
> userspace to verify those for various preexisting reasons, and Android
> recovery does not reallocate buffers to stress this issue.
> 
> Tested-by: Aaron Kling <webgeek1234@xxxxxxxxx>
> 
> Aaron

Thank you!