Re: [PATCH v4 10/12] drm/panthor: Support FW init of protm

[Boris Brezillon <boris.brezillon@collabora.com>]

On Mon, 12 Oct 2026 14:59:38 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:

> +int panthor_fw_protm_init(struct drm_file *file,
> +			  struct drm_panthor_protm_init *args)
> +{
> +	struct panthor_file *pfile = file->driver_priv;
> +	struct panthor_device *ptdev = pfile->ptdev;
> +	struct panthor_fw_section *protm_section = ptdev->fw->protm_section;
> +	struct drm_gem_object *obj;
> +	u32 vm_map_flags;
> +	int cookie, ret = 0;
> +
> +	if (!capable(CAP_SYS_MODULE))
> +		return -EPERM;
> +
> +	if (args->pad)
> +		return -EINVAL;
> +
> +	if (!protm_section || !protm_section->size)
> +		return -EINVAL;
> +
> +	guard(mutex)(&ptdev->fw->protm_init_lock);
> +
> +	/* Thread safe access so other readers do not need protm_init_lock */
> +	if (smp_load_acquire(&ptdev->protm.info.state) &
> +	    DRM_PANTHOR_PROTM_INITIALIZED)
> +		return 0;

Why not force the query to be done with the lock held instead? Querying
the PROTM state is not a hot path, so the lockless approach feels
overkill to me. If the problem is that the lock is not visible from
ptdev, then I'd argue the ptdev->protm.info shouldn't be either,
because it's not initialized at probe time. What we should have instead
is a panthor_fw_query_protm_info() helper return/filling
drm_panthor_protm_info.

> +
> +	if (!drm_dev_enter(&ptdev->base, &cookie))
> +		return -ENODEV;
> +
> +	obj = drm_gem_object_lookup(file,
> +				    args->fw_protected_sections_bo_handle);
> +	if (!obj) {
> +		ret = -ENOENT;
> +		goto out_dev_exit;
> +	}
> +
> +	if (obj->size < ptdev->protm.info.fw_protected_sections_size) {
> +		ret = -EINVAL;
> +		goto out_gem_put;
> +	}
> +
> +	vm_map_flags = section_vm_map_flags(protm_section);
> +
> +	protm_section->mem = panthor_kernel_bo_create_from_gem(
> +		ptdev, panthor_fw_vm(ptdev), to_panthor_bo(obj), vm_map_flags,
> +		protm_section->va, protm_section->size);
> +	if (IS_ERR(protm_section->mem)) {
> +		ret = PTR_ERR(protm_section->mem);
> +		protm_section->mem = NULL;
> +		goto out_gem_put;
> +	}
> +
> +	/* Make sure all above is completed before we update state */
> +	smp_store_release(&ptdev->protm.info.state,
> +			  DRM_PANTHOR_PROTM_INITIALIZED);
> +
> +out_gem_put:
> +	drm_gem_object_put(obj);
> +
> +out_dev_exit:
> +	drm_dev_exit(cookie);
> +	return ret;
> +}