public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Iago Toral <itoral@igalia.com>
To: Maíra Canal <mcanal@igalia.com>, Melissa Wen <mwen@igalia.com>,
	Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Cc: kernel-dev@igalia.com, dri-devel@lists.freedesktop.org,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 2/2] drm/v3d: Skip CSD when it has zeroed workgroups
Date: Tue, 02 Jun 2026 09:10:39 +0200	[thread overview]
Message-ID: <68bb463c77c3d65f23f53b4a2c64fd4547e45aa3.camel@igalia.com> (raw)
In-Reply-To: <20260601-v3d-fix-indirect-csd-v2-2-aaebf035b936@igalia.com>

El lun, 01-06-2026 a las 16:13 -0300, Maíra Canal escribió:
> A compute shader dispatch encodes its workgroup counts in the
> CFG0..CFG2
> registers. Kicking off a dispatch with a zero count in any of the
> three
> dimensions is invalid. First, the hardware will process 0 as 65536,
> causing an illegitimate submission. But over that, a submission with
> a
> zeroed workgroup dimension should be a no-op.
> 
> These zeroed counts can reach the dispatch path through an indirect
> CSD
> job, whose workgroup counts are only known once the indirect buffer
> is
> read and may legitimately be zero, but such scenario should only
> result in
> a no-op.
> 
> Don't submit the job to the hardware when any of the workgroup counts
> is
> zero, so the job completes immediately instead of running the shader.
> 
> Cc: stable@vger.kernel.org
> Fixes: d223f98f0209 ("drm/v3d: Add support for compute shader
> dispatch.")
> Suggested-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/v3d/v3d_sched.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c
> b/drivers/gpu/drm/v3d/v3d_sched.c
> index 47f83936cd73..681d10af4c8e 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -352,6 +352,15 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
>  		return NULL;
>  	}
>  
> +	/* For dispatch dimensions, HW interprets 0 as 65536,
> causing
> +	 * illegitimate submissions that must be rejected. Note that
> +	 * 65535 (2^16 - 1) is the maximum number of workgroups per
> dimension.
> +	 */

I am not sure this description is accurate. I think passing in 0
doesn't cause illegitimate submissions. I think the issue is that if we
allow 0 to be interpreted as 65536 then can't tell if an indirect job 
actually configured 0 as the dimension. So I'd rephrase this as:

/* The hw interprets a workgroup size of 0 as 65536, however,
 * the user-space driver exposes a maximum of 65535. So a 0 on any
 * dimension means that we have no workgroups and the compute shader
 * should not be dispatched.
 */

With that change, both patches are:

Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>

> +	if (!V3D_GET_FIELD(job->args.cfg[0],
> V3D_CSD_QUEUED_CFG0_NUM_WGS_X) ||
> +	    !V3D_GET_FIELD(job->args.cfg[1],
> V3D_CSD_QUEUED_CFG1_NUM_WGS_Y) ||
> +	    !V3D_GET_FIELD(job->args.cfg[2],
> V3D_CSD_QUEUED_CFG2_NUM_WGS_Z))
> +		return NULL;
> +
>  	v3d->queue[V3D_CSD].active_job = &job->base;
>  
>  	v3d_invalidate_caches(v3d);
> 


  reply	other threads:[~2026-06-02  7:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-01 19:13 [PATCH v2 0/2] drm/v3d: Fix indirect CSD jobs with zeroed workgroups Maíra Canal
2026-06-01 19:13 ` [PATCH v2 1/2] drm/v3d: Fix vaddr leak when indirect CSD has " Maíra Canal
2026-06-04  3:44   ` Claude review: " Claude Code Review Bot
2026-06-01 19:13 ` [PATCH v2 2/2] drm/v3d: Skip CSD when it " Maíra Canal
2026-06-02  7:10   ` Iago Toral [this message]
2026-06-04  3:44   ` Claude review: " Claude Code Review Bot
2026-06-04  3:44 ` Claude review: drm/v3d: Fix indirect CSD jobs with " Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=68bb463c77c3d65f23f53b4a2c64fd4547e45aa3.camel@igalia.com \
    --to=itoral@igalia.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jmcasanova@igalia.com \
    --cc=kernel-dev@igalia.com \
    --cc=mcanal@igalia.com \
    --cc=mwen@igalia.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox