Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Tue, 28 Nov 2023 14:31:58 +0100
From:   Maxime Ripard <mripard@kernel.org>
To:     Marco Pagani <marpagan@redhat.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Christian Koenig <christian.koenig@amd.com>,
        Javier Martinez Canillas <javierm@redhat.com>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org
Subject: Re: [PATCH v4] drm/test: add a test suite for GEM objects backed by
 shmem
Message-ID: <wmv55cgrjx343k2pvdlnigrtehv6oq7hiss3k24xmwec7hepes@riq75ckh7eh5>
References: <20231123100147.212645-1-marpagan@redhat.com>
 <dkh6t5cya7tw4wz5aqrgvnqqnyzzgvbyvezxexy6pyspju5qbd@cdl4vvs7kvnt>
 <9fb1cd9d-89cb-4f45-9d0c-9f0102e000e6@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="xa3olno3bho23d7h"
Content-Disposition: inline
In-Reply-To: <9fb1cd9d-89cb-4f45-9d0c-9f0102e000e6@redhat.com>
Precedence: bulk


--xa3olno3bho23d7h
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Nov 24, 2023 at 11:15:12AM +0100, Marco Pagani wrote:
>=20
>=20
> On 2023-11-24 09:49, Maxime Ripard wrote:
> > Hi,
> >=20
> > 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;
> >> +
> >=20
> > [...]
> >=20
> >> +
> >> +	/*
> >> +	 * 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 =3D drm_kunit_helper_alloc_drm_device(test, dev, struct fake_de=
v,
> >> +						 drm_dev, DRIVER_GEM);
> >> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev);
> >=20
> > Sorry I missed it earlier, but you don't need the intermediate structure
> > if you use
> >=20
> > struct drm_device *drm;
> >=20
> > drm =3D __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0,=
 DRIVER_GEM);
> > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
> >
>=20
> 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

--xa3olno3bho23d7h
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCZWXrzgAKCRDj7w1vZxhR
xSfVAPsF1Ca6WiaICCCMiadpBZDVn0JEV/LwVOotZsctwQGtpQEA2rrnLlTgWe0H
4ABF/XTvcd0zP3qiEHeNWwTFCT8/swE=
=SzdI
-----END PGP SIGNATURE-----

--xa3olno3bho23d7h--