Re: [05/11] drm/tests: helpers: Create an helper to allocate a locking ctx

[suijingfeng]

Hi,


On 2023/7/20 18:07, Maxime Ripard wrote:
> On Wed, Jul 19, 2023 at 09:12:14PM +0200, Javier Martinez Canillas wrote:
> > suijingfeng <suijingfeng@xxxxxxxxxxx> writes:
> >
> > > Hi,
> > >
> > > On 2023/7/10 15:47, Maxime Ripard wrote:
> > [...]
> >
> > > > +
> > > > +/**
> > > > + * drm_kunit_helper_context_alloc - Allocates an acquire context
> > > > + * @test: The test context object
> > > > + *
> > > > + * Allocates and initializes a modeset acquire context.
> > > > + *
> > > > + * The context is tied to the kunit test context, so we must not call
> > > > + * drm_modeset_acquire_fini() on it, it will be done so automatically.
> > > > + *
> > > > + * Returns:
> > > > + * An ERR_PTR on error, a pointer to the newly allocated context otherwise
> > > > + */
> > > > +struct drm_modeset_acquire_ctx *
> > > > +drm_kunit_helper_acquire_ctx_alloc(struct kunit *test)
> > > > +{
> > > > +	struct drm_modeset_acquire_ctx *ctx;
> > > > +	int ret;
> > > > +
> > > > +	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> > > > +	KUNIT_ASSERT_NOT_NULL(test, ctx);
> > > > +
> > > > +	drm_modeset_acquire_init(ctx, 0);
> > > > +
> > > > +	ret = kunit_add_action_or_reset(test,
> > > > +					action_drm_release_context,
> > > > +					ctx);
> > > > +	if (ret)
> > > > +		return ERR_PTR(ret);
> > > > +
> > > > +	return ctx;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(drm_kunit_helper_acquire_ctx_alloc);
> > > > +
> > > I think all of the patch inside this series are quite well.
> > >
> > > Personally, I can't find problems in it.
> > >
> > >
> > > But I still want to ask a question:
> > >
> > > Should the managed functions you introduced be prefixed with drmm_
> > > (instead of drm_) ?
> > That's a good question. But personally I think that the drmm_ prefix
> > should be reserved for drm_device managed resources and helpers.
> Yeah, to me drmm functions are meant for resources that are tied to the
> DRM device lifetime. These resources are tied to the test lifetime, so
> they shouldn't share the same prefix.


Okay then,

Thanks for reply.


> > > As mindless programmer may still want to call drm_modeset_acquire_fini()
> > > on the pointer returned by
> > >
> > > drm_kunit_helper_acquire_ctx_alloc()?
> > The function kernel-doc already mentions that there's no need to do that
> > and that will be done automatically by kunit. So shouldn't be different of
> > other functions helper where the programmer didn't read the documentation.
> Right
>
> Maxime