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

[suijingfeng <suijingfeng@loongson.cn>]

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@loongson.cn> 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