On Mon, Jul 15, 2024 at 04:07:57PM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Dave
> >Airlie
> >Sent: Monday, July 15, 2024 4:36 AM
> >To: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> >Subject: [PATCH] drm/test: fix the gem shmem test to map the sg table.
> >
> >From: Dave Airlie <airlied@xxxxxxxxxx>
> >
> >The test here creates an sg table, but never maps it, when
> >we get to drm_gem_shmem_free, the helper tries to unmap and this
> >causes warnings on some platforms and debug kernels.
>
> This looks pretty straightforward...
>
> However, should drm_gem_shmem_free() really give an error if the mapping
> didn't happen?
>
> I.e. just because you have an sgt pointer, should you also have a mapping?
Yes, I think only allocating an sgt but not setting it up is a bug. So the
fix looks correct, and isn't just papering over noise.
> If the errors are really just noise (form the specific platforms), and this patch is covering
> for that, then:
>
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cheers, Sima
>
> Thanks,
>
> Mike
>
> >This also sets a 64-bit dma mask, as I see an swiotlb warning if I
> >stick with the default 32-bit one.
> >
> >Fixes: 93032ae634d4 ("drm/test: add a test suite for GEM objects backed by
> >shmem")
> >Cc: stable@xxxxxxxxxxxxxxx
> >Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
> >---
> > drivers/gpu/drm/tests/drm_gem_shmem_test.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> >b/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> >index 91202e40cde9..eb3a7a84be90 100644
> >--- a/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> >+++ b/drivers/gpu/drm/tests/drm_gem_shmem_test.c
> >@@ -102,6 +102,17 @@ static void
> >drm_gem_shmem_test_obj_create_private(struct kunit *test)
> >
> > sg_init_one(sgt->sgl, buf, TEST_SIZE);
> >
> >+ /*
> >+ * Set the DMA mask to 64-bits and map the sgtables
> >+ * otherwise drm_gem_shmem_free will cause a warning
> >+ * on debug kernels.
> >+ * */
> >+ ret = dma_set_mask(drm_dev->dev, DMA_BIT_MASK(64));
> >+ KUNIT_ASSERT_EQ(test, ret, 0);
> >+
> >+ ret = dma_map_sgtable(drm_dev->dev, sgt, DMA_BIDIRECTIONAL, 0);
> >+ KUNIT_ASSERT_EQ(test, ret, 0);
> >+
> > /* Init a mock DMA-BUF */
> > buf_mock.size = TEST_SIZE;
> > attach_mock.dmabuf = &buf_mock;
> >--
> >2.45.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
