Re: [PATCH 1/2] drm/v3d: Consolidate CPU job validation in a function

[Maíra Canal]

Hi Iago,

On 12/01/26 04:38, Iago Toral wrote:

El vie, 09-01-2026 a las 15:35 -0300, Maíra Canal escribió:
(...)

[...]

@@ -770,15 +756,8 @@ v3d_get_cpu_copy_performance_query_params(struct
drm_file *file_priv,
        struct drm_v3d_copy_performance_query copy;
        int err;

- if (!job) {

-               DRM_DEBUG("CPU job extension was attached to a GPU
job.\n");
+       if (!v3d_validate_cpu_job(v3d, job))
                return -EINVAL;
-       }
-
-       if (job->job_type) {
-               DRM_DEBUG("Two CPU job extensions were added to the
same CPU job.\n");
-               return -EINVAL;
-       }

  if (copy_from_user(&copy, ext, sizeof(copy)))

                return -EFAULT;
@@ -826,6 +805,8 @@ v3d_get_extensions(struct drm_file *file_priv,
                   struct v3d_submit_ext *se,
                   struct v3d_cpu_job *job)
  {
+       struct v3d_file_priv *v3d_priv = file_priv->driver_priv;
+       struct v3d_dev *v3d = v3d_priv->v3d;

Since we can get the v3d_dev from the file_priv and we are already
passing file_priv around, does it really give us anything to also pass
the v3d_dev around if we only need it in that function? Not that I am
necessarily against it, just making sure that was a conscious decision.

Initially, I thought it would be more semantic to use v3d_dev, but
rethinking it, I can agree that it would be better to use file_priv.
I'll address it in v2.

Best regards,
- Maíra

Iago

        struct drm_v3d_extension __user *user_ext;
        int ret;

@@ -843,22 +824,22 @@ v3d_get_extensions(struct drm_file *file_priv,

                        ret =
v3d_get_multisync_submit_deps(file_priv, user_ext, se);
                        break;
                case DRM_V3D_EXT_ID_CPU_INDIRECT_CSD:
-                       ret =
v3d_get_cpu_indirect_csd_params(file_priv, user_ext, job);
+                       ret = v3d_get_cpu_indirect_csd_params(v3d,
file_priv, user_ext, job);
                        break;
                case DRM_V3D_EXT_ID_CPU_TIMESTAMP_QUERY:
-                       ret =
v3d_get_cpu_timestamp_query_params(file_priv, user_ext, job);
+                       ret =
v3d_get_cpu_timestamp_query_params(v3d, file_priv, user_ext, job);
                        break;
                case DRM_V3D_EXT_ID_CPU_RESET_TIMESTAMP_QUERY:
-                       ret =
v3d_get_cpu_reset_timestamp_params(file_priv, user_ext, job);
+                       ret =
v3d_get_cpu_reset_timestamp_params(v3d, file_priv, user_ext, job);
                        break;
                case DRM_V3D_EXT_ID_CPU_COPY_TIMESTAMP_QUERY:
-                       ret =
v3d_get_cpu_copy_query_results_params(file_priv, user_ext, job);
+                       ret =
v3d_get_cpu_copy_query_results_params(v3d, file_priv, user_ext, job);
                        break;
                case DRM_V3D_EXT_ID_CPU_RESET_PERFORMANCE_QUERY:
-                       ret =
v3d_get_cpu_reset_performance_params(file_priv, user_ext, job);
+                       ret =
v3d_get_cpu_reset_performance_params(v3d, file_priv, user_ext, job);
                        break;
                case DRM_V3D_EXT_ID_CPU_COPY_PERFORMANCE_QUERY:
-                       ret =
v3d_get_cpu_copy_performance_query_params(file_priv, user_ext, job);
+                       ret =
v3d_get_cpu_copy_performance_query_params(v3d, file_priv, user_ext,
job);
                        break;
                default:
                        DRM_DEBUG_DRIVER("Unknown extension id:
%d\n", ext.id);