Re: [PATCH v4] drm/test: add a test suite for GEM objects backed by shmem

[Maxime Ripard]

On Fri, Nov 24, 2023 at 11:15:12AM +0100, Marco Pagani wrote:
> 
> 
> On 2023-11-24 09:49, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Nov 23, 2023 at 11:01:46AM +0100, Marco Pagani wrote:
> >> +static int drm_gem_shmem_test_init(struct kunit *test)
> >> +{
> >> +	struct device *dev;
> >> +	struct fake_dev {
> >> +		struct drm_device drm_dev;
> >> +	} *fdev;
> >> +
> > 
> > [...]
> > 
> >> +
> >> +	/*
> >> +	 * The DRM core will automatically initialize the GEM core and create
> >> +	 * a DRM Memory Manager object which provides an address space pool
> >> +	 * for GEM objects allocation.
> >> +	 */
> >> +	fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev,
> >> +						 drm_dev, DRIVER_GEM);
> >> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev);
> > 
> > Sorry I missed it earlier, but you don't need the intermediate structure
> > if you use
> > 
> > struct drm_device *drm;
> > 
> > drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_GEM);
> > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
> >
> 
> I prefer to use drm_kunit_helper_alloc_drm_device() with the intermediate
> structure. It makes the code clearer, in my opinion. Initially, when
> developing the suite, I was using __drm_kunit_helper_alloc_drm_device()
> as most test suites do, but I feel the list of arguments including
> "sizeof(*drm), 0," is less straightforward to understand.

Then we can create an init helper, and you can allocate the drm_device
through drmm_kzalloc, but I'd like tests to use consistent constructs.

This can of course be made as a later patch: you use the same construct
the other tests do here, and later we work on the init function and
convert all tests to use it.

Maxime

Attachment: signature.asc
Description: PGP signature