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
