On Mon, 12 Oct 2026 14:59:38 +0200
Ketil Johnsen <[email protected]> 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;
> +}
