diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 2adf386a38d1f559ed7ef38e0487f610c4972cd9..c0ab13e627a1ecfd570c6e6fd806d5bb161fcaea 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -133,6 +133,14 @@ drm_gem_shmem_free_pages(struct drm_gem_shmem_object *shmem) { struct drm_gem_object *obj = &shmem->base; + if (shmem->sgt) { + dma_unmap_sgtable(obj->dev->dev, shmem->sgt, + DMA_BIDIRECTIONAL, 0); + sg_free_table(shmem->sgt); + kfree(shmem->sgt); + shmem->sgt = NULL; + } + #ifdef CONFIG_X86 if (shmem->map_wc) set_pages_array_wb(shmem->pages, obj->size >> PAGE_SHIFT); @@ -176,24 +184,12 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem) { struct drm_gem_object *obj = &shmem->base; - if (drm_gem_is_imported(obj)) { + if (drm_gem_is_imported(obj)) drm_prime_gem_destroy(obj, shmem->sgt); - } else { - drm_WARN_ON(obj->dev, refcount_read(&shmem->vmap_use_count)); - if (shmem->sgt) { - dma_unmap_sgtable(obj->dev->dev, shmem->sgt, - DMA_BIDIRECTIONAL, 0); - sg_free_table(shmem->sgt); - kfree(shmem->sgt); - } - if (shmem->pages && - refcount_dec_and_test(&shmem->pages_use_count)) - drm_gem_shmem_free_pages(shmem); - - drm_WARN_ON(obj->dev, refcount_read(&shmem->pages_use_count)); - drm_WARN_ON(obj->dev, refcount_read(&shmem->pages_pin_count)); - } + drm_WARN_ON(obj->dev, refcount_read(&shmem->vmap_use_count)); + drm_WARN_ON(obj->dev, refcount_read(&shmem->pages_use_count)); + drm_WARN_ON(obj->dev, refcount_read(&shmem->pages_pin_count)); drm_gem_object_release(obj); kfree(shmem); @@ -755,6 +751,9 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object *shmem) drm_WARN_ON(obj->dev, drm_gem_is_imported(obj)); + if (drm_WARN_ON(obj->dev, !shmem->pages)) + return ERR_PTR(-ENOMEM); + return drm_prime_pages_to_sg(obj->dev, shmem->pages, obj->size >> PAGE_SHIFT); } EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); @@ -770,15 +769,10 @@ static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_ drm_WARN_ON(obj->dev, drm_gem_is_imported(obj)); - ret = drm_gem_shmem_get_pages_locked(shmem); - if (ret) - return ERR_PTR(ret); - sgt = drm_gem_shmem_get_sg_table(shmem); - if (IS_ERR(sgt)) { - ret = PTR_ERR(sgt); - goto err_put_pages; - } + if (IS_ERR(sgt)) + return sgt; + /* Map the pages for use by the h/w. */ ret = dma_map_sgtable(obj->dev->dev, sgt, DMA_BIDIRECTIONAL, 0); if (ret) @@ -791,8 +785,6 @@ static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_ err_free_sgt: sg_free_table(sgt); kfree(sgt); -err_put_pages: - drm_gem_shmem_put_pages_locked(shmem); return ERR_PTR(ret); } @@ -809,6 +801,17 @@ static struct sg_table *drm_gem_shmem_get_pages_sgt_locked(struct drm_gem_shmem_ * and difference between dma-buf imported and natively allocated objects. * drm_gem_shmem_get_sg_table() should not be directly called by drivers. * + * Drivers should adhere to these SGT usage rules: + * + * 1. SGT should be allocated only if shmem pages are pinned at the + * time of allocation, otherwise allocation will fail. + * + * 2. Drivers should ensure that pages are pinned during the time of + * SGT usage and should get new SGT if pages were unpinned. + * + * Drivers don't own returned SGT and must take care of the SGT pointer + * lifetime. SGT is valid as long as GEM pages that backing SGT are pinned. + * * Returns: * A pointer to the scatter/gather table of pinned pages or errno on failure. */ diff --git a/drivers/gpu/drm/tests/drm_gem_shmem_test.c b/drivers/gpu/drm/tests/drm_gem_shmem_test.c index 81cadaecdd4f6ed65945b56e9f33263af32d848f..f1a8ffb9dc2666170d1bdc6597b89c270ed35727 100644 --- a/drivers/gpu/drm/tests/drm_gem_shmem_test.c +++ b/drivers/gpu/drm/tests/drm_gem_shmem_test.c @@ -34,6 +34,9 @@ KUNIT_DEFINE_ACTION_WRAPPER(sg_free_table_wrapper, sg_free_table, KUNIT_DEFINE_ACTION_WRAPPER(drm_gem_shmem_free_wrapper, drm_gem_shmem_free, struct drm_gem_shmem_object *); +KUNIT_DEFINE_ACTION_WRAPPER(drm_gem_shmem_put_pages_wrapper, drm_gem_shmem_put_pages, + struct drm_gem_shmem_object *); + /* * Test creating a shmem GEM object backed by shmem buffer. The test * case succeeds if the GEM object is successfully allocated with the @@ -247,6 +250,12 @@ static void drm_gem_shmem_test_get_sg_table(struct kunit *test) ret = kunit_add_action_or_reset(test, drm_gem_shmem_free_wrapper, shmem); KUNIT_ASSERT_EQ(test, ret, 0); + ret = drm_gem_shmem_get_pages(shmem); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = kunit_add_action_or_reset(test, drm_gem_shmem_put_pages_wrapper, shmem); + KUNIT_ASSERT_EQ(test, ret, 0); + /* The scatter/gather table will be freed by drm_gem_shmem_free */ sgt = drm_gem_shmem_get_pages_sgt(shmem); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sgt); @@ -322,6 +331,12 @@ static void drm_gem_shmem_test_purge(struct kunit *test) ret = drm_gem_shmem_madvise_locked(shmem, 1); KUNIT_EXPECT_TRUE(test, ret); + ret = drm_gem_shmem_get_pages(shmem); + KUNIT_ASSERT_EQ(test, ret, 0); + + ret = kunit_add_action_or_reset(test, drm_gem_shmem_put_pages_wrapper, shmem); + KUNIT_ASSERT_EQ(test, ret, 0); + /* The scatter/gather table will be freed by drm_gem_shmem_free */ sgt = drm_gem_shmem_get_pages_sgt(shmem); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sgt);