Re: [PATCH v2 07/10] drm/nouveau/disp: fix head state readback on GB20x

One nitpick below

On Thu, 2026-08-20 at 20:49 +0400, Mohamed Ahmed wrote:
> The GSP path reads armed head state and the RG scanout position
> through
> gv100_head_state() and gv100_head_rgpos() on every generation.
> gv100_head_state() reads the core channel's state mirror at a 0x400
> per-head stride, which NVD5.0 (GB20x) doubled. Per NVIDIA's published
> CA7D class header every HEAD_SET method sits at 0x2000 + head *
> 0x800,
> while the mirror bases are unchanged (assembly at 0x680000, armed at
> +0x8000, per OpenRM's v03_00 channel-user-base HAL which is still
> used on
> DISPv0502).
> 
> Add gb202_head_state(), the same readback at the 0x800 stride, and
> supply it through gb202_gsp_disp.
> gv100_head_rgpos() is kept. The RG registers keep their per-head
> 0x800
> stride on NVD5.0, and OpenRM's
> kdispReadRgLineCountAndFrameCount_v03_00
> still reads NV_PDISP_RG_DPCA on DISPv0502.
> 
> Signed-off-by: Mohamed Ahmed <[email protected]>
> ---
>  .../gpu/drm/nouveau/nvkm/engine/disp/gb202.c  | 49
> ++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c
> index 4863b2b36db0..a66c820be9fe 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c
> @@ -83,6 +83,53 @@ gb202_sor_hdmi_gcp(struct nvkm_ior *sor, int head,
> bool enable)
>  	nvkm_mask(device, 0x6f0040 + hdmi, 0x00000001, 0x00000001);
>  }
>  
> +/* Same core-channel state mirror as gv100_head_state() (assembly at
> 0x680000,
> + * armed at +0x8000, per-head method offsets unchanged), but NVD5.0
> spaces
> + * heads 0x800 apart (see NVCA7D_HEAD_SET_*(a) in clca7d.h).
> + */
> +static void
> +gb202_head_state(struct nvkm_head *head, struct nvkm_head_state
> *state)
> +{
> +	struct nvkm_device *device = head->disp-
> >engine.subdev.device;
> +	const u32 hoff = (state == &head->arm) * 0x8000 + head->id *
> 0x800;
> +	u32 data;
> +
> +	data = nvkm_rd32(device, 0x682064 + hoff);
> +	state->vtotal = (data & 0xffff0000) >> 16;
> +	state->htotal = (data & 0x0000ffff);
> +	data = nvkm_rd32(device, 0x682068 + hoff);
> +	state->vsynce = (data & 0xffff0000) >> 16;
> +	state->hsynce = (data & 0x0000ffff);
> +	data = nvkm_rd32(device, 0x68206c + hoff);
> +	state->vblanke = (data & 0xffff0000) >> 16;
> +	state->hblanke = (data & 0x0000ffff);
> +	data = nvkm_rd32(device, 0x682070 + hoff);
> +	state->vblanks = (data & 0xffff0000) >> 16;
> +	state->hblanks = (data & 0x0000ffff);
> +	/* Bit 31 is ADJ1000DIV1001, not a HERTZ bit. We don't have
> enough bits
> +	 * to add the full clock in hz on Blackwell (35 bits), but
> state->hz
> +	 * is unused and obsolete under GSP so this is fine.
> +	 */
> +	state->hz = nvkm_rd32(device, 0x68200c + hoff) & 0x7fffffff;
> +
> +	data = nvkm_rd32(device, 0x682004 + hoff);
> +	switch ((data & 0x000000f0) >> 4) {
> +	case 5:
> +		state->or.depth = 30;
> +		break;
> +	case 4:
> +		state->or.depth = 24;
> +		break;
> +	case 1:
> +		state->or.depth = 18;
> +		break;
> +	default:
> +		state->or.depth = 18;
> +		WARN_ON(1);
> +		break;
> +	}

I would probably condense this switch case like ben did with
gv100_head_state():

	switch ((data & 0x000000f0) >> 4) {
	case 5: state->or.depth = 30; break;
	case 4: state->or.depth = 24; break;
	case 1: state->or.depth = 18; break;
	default:
		state->or.depth = 18;
		WARN_ON(1);
		break;
	}

With that fixed:

Reviewed-by: Lyude Paul <[email protected]>

> +}
> +
>  /* GB20x is GSP-only. This table supplies the register programming
> the
>   * GSP-RM display path needs from the chip.
>   */
> @@ -91,7 +138,7 @@ gb202_gsp_disp = {
>  	.uevent = &gv100_disp_chan_uevent,
>  	.ramht_size = 0x2000,
>  	.gsp.intr = tu102_disp_intr,
> -	.gsp.head_state = gv100_head_state,
> +	.gsp.head_state = gb202_head_state,
>  	.gsp.head_rgpos = gv100_head_rgpos,
>  	.gsp.vblank_get = tu102_head_vblank_get,
>  	.gsp.vblank_put = tu102_head_vblank_put,