From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 738DB37A486 for ; Fri, 21 Aug 2026 21:30:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787347845; cv=none; b=Yyz7/rlLr+XWI5hDghyJ0vfDcPzJMQTIc11BqK+es/CpW1R3jdJS2IfGYNNguzYgj9AA0pCDVnLGX8nMn5OlFAN6ENopgIyQRDPIqjZpanBBxSk0pA5IAa2VgxG18LcMF4Ck7Ks0QR5wFjmXeakhC2+cpj/MLr4rvpuufB4iguQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787347845; c=relaxed/simple; bh=e8JQsKqnQNW0MMumwkk3a1IeHbapZmtEWm1461aYL44=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=e4+ahckGWfAKQws1El7bwa+qk6gAYBnMekna+/kKCnbH3amzvEjDAex0iUbH/hpdQHR4GGbv8w1y+ttRtzMrNn6g+exW5Lwk/q8ZZIWixGVRqT8+mCpkwEWrtGFtI5d65u+D6wTK5ucW+wmVf+/5fiipDbSnwav5dqSKgtSrQtc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=fRKb0gm2; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=qj8NFIIF; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="fRKb0gm2"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="qj8NFIIF" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1787347842; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jUTgNefrV0RVSYcbLese14a3Jj7kRytHJFAkoezSnU4=; b=fRKb0gm2geHj+g6BTSiaVCs8snTG2OHSIwNiruu9hiom7FCklwZkSd9hHhfL2Aso3pL49P p0yrYZmh/YD47wHMImTEs/VhQOVxw539L7eDOCNQM3kRpjMU0bCzpj28hj9dvkDhs9WCtp 8Olg+6HVfMGe0cXWQoSl3Ipw8NCn/VE= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-304-7kGfGRhbPmW0M7jOre8vrw-1; Fri, 21 Aug 2026 17:30:40 -0400 X-MC-Unique: 7kGfGRhbPmW0M7jOre8vrw-1 X-Mimecast-MFC-AGG-ID: 7kGfGRhbPmW0M7jOre8vrw_1787347840 Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-8eec6acbe21so26660786d6.3 for ; Fri, 21 Aug 2026 14:30:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1787347840; x=1787952640; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:content-type :references:in-reply-to:date:cc:to:from:subject:message-id:from:to :cc:subject:date:message-id:reply-to:content-type; bh=jUTgNefrV0RVSYcbLese14a3Jj7kRytHJFAkoezSnU4=; b=qj8NFIIFiq4aZ+OXS68Jng11gzJhw4Kh2T5W/9LK6Lz881SrFl+RYWSMpk1U276O7E U2YFO3lXgnjHjJR039k6Nns0Cr8RGEw3TjaBrEdl4KN8hkieNJo0B/kEExNercqyo8Eu CKcUy2ECLxok4P+r+HBCHCdKvmQ+b0mSZTs+GIMVIYtVHKZWNSZpH2Jv2XczwT05vmRN 0E+L3sa582ebeWxL/J4LOpn0letSF9DObIxHnVS86umRUZiyem6xf+oEhBHO/j/j7Zfk kCTlwdHzcnvJGw/tmUgb/jQ5hVjX7uTdUxjftNMocXtVed1+SJUZWzRNno+QcM9RO7/2 WK9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787347840; x=1787952640; h=mime-version:user-agent:content-transfer-encoding:content-type :references:in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=jUTgNefrV0RVSYcbLese14a3Jj7kRytHJFAkoezSnU4=; b=XT1re85VMJ6LsF0lGFQm/qkQGXDS87tggC4CgK10xmPnv/JxESn7WREOEHh4V0VZyi ADJTkg2HVkqSoaI7i0+wsoNM5S+nl6JvJ31FwqZn0AZ9F86a7hSpHdFANE56+uU0Uzeb HvRwhLcJ5p38TUVJW8QgUvodTz3cR0p3gFqMu3RIMXYoJVzMYxRbWraIQwPmXYXcfjpg n5Y6lRpSu7SVyt/gM9kh5m9rzSx2VVqAe4Tj/E85TQR0ylUH2YY0bHeDzRBiX2AzwL2j tZDz7gPUCsyifxbC7eAZes9QftiN3fljmldZ8SVebGSGhsBnisMawvw8Hrol2h0PCedE mhtw== X-Forwarded-Encrypted: i=1; AHgh+RpiUhIaxHwSY8j22DYd1Urr66viySW6SBUR79+o6kC6FJLdWzAuLTF4Pl60pL8CaG5s3kKdoz8z56JBpng=@vger.kernel.org X-Gm-Message-State: AFuF++m+/pMzsX21ZxXHFOVIAndcLd3Rzo8ouH6Y9cgtm183IGZKZkK9 4t7mFrSFKkh2g/spxuQNMcVDEwUzQ2Y7cBsSDysqzhF/tOlNrrGQCKMjJcg7KxqFU943osJ4CYd Q1YRPF2Bt1agCXm0PZNZuKMC1OCioLxxlr6fbAGrsYQNllF/S4e5LfTScR4tZoAaFBQ== X-Gm-Gg: AR+sD11+FBryM/vGo1ZSJfZ4WEx/5EeAt2P4PxfWr2xz6y8/cZvmfUrV7iOILKt7HuB O6zWSSJ+3vTyUIyj1IUnnWMT13XSXTxmmRAOGnretFOOkPAFUfy1PN9mZnMocwRClhvACIfwaNu b8BvoEcxCl1/5aR5iFwEOs3gGurZ61L7MQX7HIsCZ0KfBkZxROyhuHuJ8x7WeNSW5GFLnuhAG7L xJAej2AcSEdndDSlhRsDY1OGBYapQL1OdL4GUjpkolhfTVd3Q6HQ7m6MzqWff5beP+DZFUj6wOi HcQ4sq3fNTKKzxhE0zES2c7iqXwxa4TMdTPsE5TcRFDOjxj010M0UrgkCr+LObVKSWQ/dr0a X-Received: by 2002:a05:6214:2686:b0:8f4:821d:95f7 with SMTP id 6a1803df08f44-90c8f9fd3a3mr14377886d6.18.1787347839830; Fri, 21 Aug 2026 14:30:39 -0700 (PDT) X-Received: by 2002:a05:6214:2686:b0:8f4:821d:95f7 with SMTP id 6a1803df08f44-90c8f9fd3a3mr14376976d6.18.1787347838764; Fri, 21 Aug 2026 14:30:38 -0700 (PDT) Received: from [192.168.8.4] ([100.0.180.93]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-90c5eec934bsm72659966d6.15.2026.08.21.14.30.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Aug 2026 14:30:36 -0700 (PDT) Message-ID: <9b9a3f93ccaa64efa6f24ebd24d933ee758702e6.camel@redhat.com> Subject: Re: [PATCH v2 03/10] drm/nouveau/disp: route GSP-RM display MMIO through nvkm_disp_func hooks From: lyude@redhat.com To: Mohamed Ahmed , linux-kernel@vger.kernel.org Cc: dri-devel@lists.freedesktop.org, Danilo Krummrich , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Mary Guillemard , nouveau@lists.freedesktop.org Date: Fri, 21 Aug 2026 17:30:36 -0400 In-Reply-To: <20260820164929.17117-4-mohamedahmedegypt2001@gmail.com> References: <20260820164929.17117-1-mohamedahmedegypt2001@gmail.com> <20260820164929.17117-4-mohamedahmedegypt2001@gmail.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Some comments below: On Thu, 2026-08-20 at 20:49 +0400, Mohamed Ahmed wrote: > The GSP-RM display code in rm/r535/disp.c borrows a few > register-programming routines from engine/disp (the head-timing > interrupt handler, vblank enables, armed head state and scanout > position > readback, the AVI/VSI infoframe writers and the GCP AVMute write) and > so > far picked them by name, which means it has to know which chip it > runs > on the moment a generation changes any of them. >=20 > Give nvkm_disp_func a .gsp table that each chip fills with exactly > those > hooks, add tu102_gsp_disp (TU1xx) and ga102_gsp_disp (GA10x onwards) > carrying the current functions, hand them to r535_disp_new() instead > of > the full hardware tables, and make rm/r535/disp.c call through the > hooks. r535_head becomes four forwarders, r535_sor_hdmi gets > infoframe > forwarders, r535_sor_hdmi_audio() calls the GCP hook, and the > interrupt > handler and its vector come from the table (intr_low_latency selects > the > second DISP interrupt instance for chips that raise head timing on a > separate vector). The tables are per chip even though the two > currently > coincide, so a generation that changes a hook only touches its own > file. > rm/r535/disp.c no longer contains chip-specific register code, and a > new > display generation only has to provide its own table. No functional > change. >=20 > Signed-off-by: Mohamed Ahmed > --- > =C2=A0.../gpu/drm/nouveau/nvkm/engine/disp/ga102.c=C2=A0 | 16 ++++- > =C2=A0.../gpu/drm/nouveau/nvkm/engine/disp/priv.h=C2=A0=C2=A0 | 19 ++++++ > =C2=A0.../gpu/drm/nouveau/nvkm/engine/disp/tu102.c=C2=A0 | 16 ++++- > =C2=A0.../nouveau/nvkm/subdev/gsp/rm/r535/disp.c=C2=A0=C2=A0=C2=A0 | 60 += +++++++++++++++- > -- > =C2=A04 files changed, 100 insertions(+), 11 deletions(-) >=20 > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/ga102.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/ga102.c > index ab0a85c92430..b48ed7146396 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/ga102.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/ga102.c > @@ -144,12 +144,26 @@ ga102_disp =3D { > =C2=A0 }, > =C2=A0}; > =C2=A0 > +static const struct nvkm_disp_func > +ga102_gsp_disp =3D { > + .uevent =3D &gv100_disp_chan_uevent, > + .ramht_size =3D 0x2000, > + .gsp.intr =3D tu102_disp_intr, > + .gsp.head_state =3D gv100_head_state, > + .gsp.head_rgpos =3D gv100_head_rgpos, > + .gsp.vblank_get =3D tu102_head_vblank_get, > + .gsp.vblank_put =3D tu102_head_vblank_put, > + .gsp.hdmi_gcp =3D tu102_sor_hdmi_gcp, > + .gsp.hdmi_infoframe_avi =3D gv100_sor_hdmi_infoframe_avi, > + .gsp.hdmi_infoframe_vsi =3D gv100_sor_hdmi_infoframe_vsi, > +}; > + > =C2=A0int > =C2=A0ga102_disp_new(struct nvkm_device *device, enum nvkm_subdev_type > type, int inst, > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct nvkm_disp **pdisp) > =C2=A0{ > =C2=A0 if (nvkm_gsp_rm(device->gsp)) > - return r535_disp_new(&ga102_disp, device, type, > inst, pdisp); > + return r535_disp_new(&ga102_gsp_disp, device, type, > inst, pdisp); > =C2=A0 > =C2=A0 return nvkm_disp_new_(&ga102_disp, device, type, inst, > pdisp); > =C2=A0} > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/priv.h > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/priv.h > index 722ec340e12a..3cb903741fb8 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/priv.h > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/priv.h > @@ -5,6 +5,8 @@ > =C2=A0#include > =C2=A0#include > =C2=A0struct nvkm_head; > +struct nvkm_head_state; > +struct nvkm_ior; > =C2=A0struct nvkm_outp; > =C2=A0struct dcb_output; > =C2=A0 > @@ -34,6 +36,23 @@ struct nvkm_disp_func { > =C2=A0 int (*new)(struct nvkm_disp *, int id); > =C2=A0 } wndw, head, dac, sor, pior; > =C2=A0 > + /* Register programming that the GSP-RM display path > (rm/r535) needs from > + * the chip, everything else on that path goes through RM. > Every hook > + * is called unconditionally. > + */ > + struct { > + irqreturn_t (*intr)(struct nvkm_inth *); > + /* Head-timing interrupts arrive on a second DISP > vector. */ > + bool intr_low_latency; > + void (*head_state)(struct nvkm_head *, struct > nvkm_head_state *); > + void (*head_rgpos)(struct nvkm_head *, u16 *hline, > u16 *vline); This looks mostly fine. As far as I can tell though, it seems like there's no actual behavioral differences between the gsp's head_state and the non-GSP head_state, same for head_rgpos. Is it possible for us to drop these two callbacks and keep using nvkm_head_func for that? Perhaps by having a second nvkm_head_func that we call back down to from RM's? FWIW by the way, I think if we end up with say - a nvkm_head_func struct that only has head_state/head_rgpos filled and nothing else (e.g. using it without GSP would break things) that's probably fine since booting these cards without GSP isn't possible on nouveau anyhow. > + void (*vblank_get)(struct nvkm_head *); > + void (*vblank_put)(struct nvkm_head *); > + void (*hdmi_gcp)(struct nvkm_ior *, int head, bool > enable); > + void (*hdmi_infoframe_avi)(struct nvkm_ior *, int > head, void *data, u32 size); > + void (*hdmi_infoframe_vsi)(struct nvkm_ior *, int > head, void *data, u32 size); > + } gsp; > + > =C2=A0 u16 ramht_size; > =C2=A0 > =C2=A0 struct nvkm_sclass root; > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/tu102.c > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/tu102.c > index 6cfd52c9056f..9db3cac487e3 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/tu102.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/tu102.c > @@ -295,12 +295,26 @@ tu102_disp =3D { > =C2=A0 }, > =C2=A0}; > =C2=A0 > +static const struct nvkm_disp_func > +tu102_gsp_disp =3D { > + .uevent =3D &gv100_disp_chan_uevent, > + .ramht_size =3D 0x2000, > + .gsp.intr =3D tu102_disp_intr, > + .gsp.head_state =3D gv100_head_state, > + .gsp.head_rgpos =3D gv100_head_rgpos, > + .gsp.vblank_get =3D tu102_head_vblank_get, > + .gsp.vblank_put =3D tu102_head_vblank_put, > + .gsp.hdmi_gcp =3D tu102_sor_hdmi_gcp, > + .gsp.hdmi_infoframe_avi =3D gv100_sor_hdmi_infoframe_avi, > + .gsp.hdmi_infoframe_vsi =3D gv100_sor_hdmi_infoframe_vsi, > +}; > + > =C2=A0int > =C2=A0tu102_disp_new(struct nvkm_device *device, enum nvkm_subdev_type > type, int inst, > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct nvkm_disp **pdisp) > =C2=A0{ > =C2=A0 if (nvkm_gsp_rm(device->gsp)) > - return r535_disp_new(&tu102_disp, device, type, > inst, pdisp); > + return r535_disp_new(&tu102_gsp_disp, device, type, > inst, pdisp); > =C2=A0 > =C2=A0 return nvkm_disp_new_(&tu102_disp, device, type, inst, > pdisp); > =C2=A0} > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c > index cd4451e62512..f3e55253bcbc 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c > @@ -547,7 +547,19 @@ r535_sor_hdmi_audio(struct nvkm_ior *sor, int > head, bool enable) > =C2=A0{ > =C2=A0 r535_sor_hdmi_ctrl_audio(sor->asy.outp, enable); > =C2=A0 r535_sor_hdmi_ctrl_audio_mute(sor->asy.outp, !enable); > - tu102_sor_hdmi_gcp(sor, head, enable); > + sor->disp->func->gsp.hdmi_gcp(sor, head, enable); > +} > + > +static void > +r535_sor_hdmi_infoframe_avi(struct nvkm_ior *sor, int head, void > *data, u32 size) > +{ > + sor->disp->func->gsp.hdmi_infoframe_avi(sor, head, data, > size); > +} > + > +static void > +r535_sor_hdmi_infoframe_vsi(struct nvkm_ior *sor, int head, void > *data, u32 size) > +{ > + sor->disp->func->gsp.hdmi_infoframe_vsi(sor, head, data, > size); > =C2=A0} > =C2=A0 > =C2=A0static void > @@ -575,8 +587,8 @@ r535_sor_hdmi =3D { > =C2=A0 .ctrl =3D r535_sor_hdmi_ctrl, > =C2=A0 .scdc =3D r535_sor_hdmi_scdc, > =C2=A0 /*TODO: SF_USER -> KMS. */ > - .infoframe_avi =3D gv100_sor_hdmi_infoframe_avi, > - .infoframe_vsi =3D gv100_sor_hdmi_infoframe_vsi, > + .infoframe_avi =3D r535_sor_hdmi_infoframe_avi, > + .infoframe_vsi =3D r535_sor_hdmi_infoframe_vsi, > =C2=A0 .audio =3D r535_sor_hdmi_audio, > =C2=A0}; > =C2=A0 > @@ -601,12 +613,36 @@ r535_sor_cnt(struct nvkm_disp *disp, unsigned > long *pmask) > =C2=A0 return 4; > =C2=A0} > =C2=A0 > +static void > +r535_head_state(struct nvkm_head *head, struct nvkm_head_state > *state) > +{ > + head->disp->func->gsp.head_state(head, state); > +} > + > +static void > +r535_head_rgpos(struct nvkm_head *head, u16 *hline, u16 *vline) > +{ > + head->disp->func->gsp.head_rgpos(head, hline, vline); > +} > + > +static void > +r535_head_vblank_get(struct nvkm_head *head) > +{ > + head->disp->func->gsp.vblank_get(head); > +} > + > +static void > +r535_head_vblank_put(struct nvkm_head *head) > +{ > + head->disp->func->gsp.vblank_put(head); > +} > + > =C2=A0static const struct nvkm_head_func > =C2=A0r535_head =3D { > - .state =3D gv100_head_state, > - .rgpos =3D gv100_head_rgpos, > - .vblank_get =3D tu102_head_vblank_get, > - .vblank_put =3D tu102_head_vblank_put, > + .state =3D r535_head_state, > + .rgpos =3D r535_head_rgpos, > + .vblank_get =3D r535_head_vblank_get, > + .vblank_put =3D r535_head_vblank_put, > =C2=A0}; > =C2=A0 > =C2=A0static struct nvkm_conn * > @@ -1650,12 +1686,17 @@ r535_disp_oneinit(struct nvkm_disp *disp) > =C2=A0 if (ret) > =C2=A0 return ret; > =C2=A0 > - ret =3D nvkm_gsp_intr_stall(gsp, disp->engine.subdev.type, > disp->engine.subdev.inst); > + /* Chips that raise head-timing interrupts on a separate > low-latency > + * vector report it as a second DISP interrupt table entry, > exposed > + * as instance 1 by the RM engine-index translation. > + */ > + ret =3D nvkm_gsp_intr_stall(gsp, disp->engine.subdev.type, > + =C2=A0 disp->func->gsp.intr_low_latency ? > 1 : disp->engine.subdev.inst); > =C2=A0 if (ret < 0) > =C2=A0 return ret; > =C2=A0 > =C2=A0 ret =3D nvkm_inth_add(&device->vfn->intr, ret, > NVKM_INTR_PRIO_NORMAL, &disp->engine.subdev, > - =C2=A0=C2=A0=C2=A0 tu102_disp_intr, &disp- > >engine.subdev.inth); > + =C2=A0=C2=A0=C2=A0 disp->func->gsp.intr, &disp- > >engine.subdev.inth); > =C2=A0 if (ret) > =C2=A0 return ret; > =C2=A0 > @@ -1688,6 +1729,7 @@ r535_disp_new(const struct nvkm_disp_func *hw, > struct nvkm_device *device, > =C2=A0 rm->uevent =3D hw->uevent; > =C2=A0 rm->sor.cnt =3D r535_sor_cnt; > =C2=A0 rm->sor.new =3D r535_sor_new; > + rm->gsp =3D hw->gsp; > =C2=A0 rm->ramht_size =3D hw->ramht_size; > =C2=A0 > =C2=A0 rm->root.oclass =3D gpu->disp.class.root;