Re: [PATCH 4/7] nouveau/gsp: use per-version DP_CONFIG_STREAM params on r570 firmware

From: lyude

Date: Mon Aug 17 2026 - 18:05:32 EST

On Sat, 2026-08-15 at 03:57 +0400, Mohamed Ahmed wrote:
> NVIDIA removed the deprecated actualPclkHz/linkClkFreqHz fields and
> the
> whole Legacy{activeCnt, activeFrac, activePolarity, mvidWarEnabled,
> MvidWarParams} block from the SST sub-struct of
> NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS between the 535 and 570
> releases
> (compared in OpenRM tags 535.113.01 vs 570.144), shrinking the
> struct.
>
> Everything nouveau writes sits at identical offsets in both layouts
> but
> the size is wrong on r570, which means r535_sor_dp_sst() and
> r535_sor_dp_vcpi() are sent with an incorrect size.
>
> Route the .sst/.vcpi IOR functions through nvkm_rm_api_disp the same
> way
> bl_ctrl and dp.get_caps/set_indexed_link_rates already are. Keep the
> existing implementation for r535 and add an r570 implementation built
> against the 570.144 layout, which already exists in r570/nvrm/disp.h
> but
> was unused until now. Also add the NV0073_CTRL_CMD_DP_CONFIG_STREAM
> define that was missing from the layout.
>
> Other DP controls sent through shared r535 code did not change layout
> between the tags.
>
> Signed-off-by: Mohamed Ahmed <mohamedahmedegypt2001@xxxxxxxxx>
> ---
> .../nouveau/nvkm/subdev/gsp/rm/r535/disp.c    | 25 +++++++-
> .../nouveau/nvkm/subdev/gsp/rm/r570/disp.c    | 64
> +++++++++++++++++++
> .../nvkm/subdev/gsp/rm/r570/nvrm/disp.h       | 2 +
> .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h   | 5 ++
> 4 files changed, 93 insertions(+), 3 deletions(-)
>
> 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 f51ad7c754d8..dd632767aea5 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
> @@ -403,7 +403,7 @@ r535_sor_dp_audio(struct nvkm_ior *sor, int head,
> bool enable)
> }
>
> static void
> -r535_sor_dp_vcpi(struct nvkm_ior *sor, int head, u8 slot, u8
> slot_nr, u16 pbn, u16 aligned_pbn)
> +r535_dp_vcpi(struct nvkm_ior *sor, int head, u8 slot, u8 slot_nr,
> u16 pbn, u16 aligned_pbn)
> {
> struct nvkm_disp *disp = sor->disp;
> struct NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS *ctrl;
> @@ -434,9 +434,17 @@ r535_sor_dp_vcpi(struct nvkm_ior *sor, int head,
> u8 slot, u8 slot_nr, u16 pbn, u
> WARN_ON(nvkm_gsp_rm_ctrl_wr(&disp->rm.objcom, ctrl));
> }
>
> +static void
> +r535_sor_dp_vcpi(struct nvkm_ior *sor, int head, u8 slot, u8
> slot_nr, u16 pbn, u16 aligned_pbn)
> +{
> + const struct nvkm_rm_api *rmapi = sor->disp-
> >engine.subdev.device->gsp->rm->api;
> +
> + rmapi->disp->dp.vcpi(sor, head, slot, slot_nr, pbn,
> aligned_pbn);
> +}
> +
> static int
> -r535_sor_dp_sst(struct nvkm_ior *sor, int head, bool ef,
> - u32 watermark, u32 hblanksym, u32 vblanksym)
> +r535_dp_sst(struct nvkm_ior *sor, int head, bool ef,
> +     u32 watermark, u32 hblanksym, u32 vblanksym)
> {
> struct nvkm_disp *disp = sor->disp;
> struct NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS *ctrl;
> @@ -463,6 +471,15 @@ r535_sor_dp_sst(struct nvkm_ior *sor, int head,
> bool ef,
> return nvkm_gsp_rm_ctrl_wr(&disp->rm.objcom, ctrl);
> }
>
> +static int
> +r535_sor_dp_sst(struct nvkm_ior *sor, int head, bool ef,
> + u32 watermark, u32 hblanksym, u32 vblanksym)
> +{
> + const struct nvkm_rm_api *rmapi = sor->disp-
> >engine.subdev.device->gsp->rm->api;
> +
> + return rmapi->disp->dp.sst(sor, head, ef, watermark,
> hblanksym, vblanksym);
> +}
> +
> static const struct nvkm_ior_func_dp
> r535_sor_dp = {
> .sst = r535_sor_dp_sst,
> @@ -1828,6 +1845,8 @@ r535_disp = {
> .dp = {
> .get_caps = r535_dp_get_caps,
> .set_indexed_link_rates =
> r535_dp_set_indexed_link_rates,
> + .sst = r535_dp_sst,
> + .vcpi = r535_dp_vcpi,
> },
> .chan = {
> .set_pushbuf = r535_disp_chan_set_pushbuf,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/disp.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/disp.c
> index a96e31c2d80b..61f82e15ff39 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/disp.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/disp.c
> @@ -5,6 +5,7 @@
> #include <rm/rm.h>
>
> #include <engine/disp.h>
> +#include <engine/disp/ior.h>
> #include <engine/disp/outp.h>
>
> #include "nvhw/drf.h"
> @@ -74,6 +75,67 @@ r570_disp_chan_set_pushbuf(struct nvkm_disp *disp,
> s32 oclass, int inst, struct
> return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice,
> ctrl);
> }
>
> +static void
> +r570_dp_vcpi(struct nvkm_ior *sor, int head, u8 slot, u8 slot_nr,
> u16 pbn, u16 aligned_pbn)
> +{
> + struct nvkm_disp *disp = sor->disp;
> + NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS *ctrl;
> +
> + ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
> +
> NV0073_CTRL_CMD_DP_CONFIG_STREAM, sizeof(*ctrl));
> + if (WARN_ON(IS_ERR(ctrl)))
> + return;
> +
> + ctrl->subDeviceInstance = 0;
> + ctrl->head = head;
> + ctrl->sorIndex = sor->id;
> + ctrl->dpLink = sor->asy.link == 2;
> + ctrl->bEnableOverride = 1;
> + ctrl->bMST = 1;
> + ctrl->hBlankSym = 0;
> + ctrl->vBlankSym = 0;
> + ctrl->colorFormat = 0;
> + ctrl->bEnableTwoHeadOneOr = 0;
> + ctrl->singleHeadMultistreamMode = 0;
> + ctrl->MST.slotStart = slot;
> + ctrl->MST.slotEnd = slot + slot_nr - 1;
> + ctrl->MST.PBN = pbn;
> + ctrl->MST.Timeslice = aligned_pbn;
> + ctrl->MST.sendACT = 0;
> + ctrl->MST.singleHeadMSTPipeline = 0;
> + ctrl->MST.bEnableAudioOverRightPanel = 0;
> + WARN_ON(nvkm_gsp_rm_ctrl_wr(&disp->rm.objcom, ctrl));
> +}

Should we maybe just update this function so that it has a return value
rather than doing the WARN_ONs explicitly under the HAL? Seems more in
line with what we do elsewhere, even if the only action the caller ends
up taking in response is printing a kernel warning.

> +
> +static int
> +r570_dp_sst(struct nvkm_ior *sor, int head, bool ef,
> +     u32 watermark, u32 hblanksym, u32 vblanksym)
> +{
> + struct nvkm_disp *disp = sor->disp;
> + NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS *ctrl;
> +
> + ctrl = nvkm_gsp_rm_ctrl_get(&disp->rm.objcom,
> +
> NV0073_CTRL_CMD_DP_CONFIG_STREAM, sizeof(*ctrl));
> + if (IS_ERR(ctrl))
> + return PTR_ERR(ctrl);
> +
> + ctrl->subDeviceInstance = 0;
> + ctrl->head = head;
> + ctrl->sorIndex = sor->id;
> + ctrl->dpLink = sor->asy.link == 2;
> + ctrl->bEnableOverride = 1;
> + ctrl->bMST = 0;
> + ctrl->hBlankSym = hblanksym;
> + ctrl->vBlankSym = vblanksym;
> + ctrl->colorFormat = 0;
> + ctrl->bEnableTwoHeadOneOr = 0;
> + ctrl->SST.bEnhancedFraming = ef;
> + ctrl->SST.tuSize = 64;
> + ctrl->SST.waterMark = watermark;
> + ctrl->SST.bEnableAudioOverRightPanel = 0;
> + return nvkm_gsp_rm_ctrl_wr(&disp->rm.objcom, ctrl);
> +}
> +
> static int
> r570_dp_set_indexed_link_rates(struct nvkm_outp *outp)
> {
> @@ -255,6 +317,8 @@ r570_disp = {
> .dp = {
> .get_caps = r570_dp_get_caps,
> .set_indexed_link_rates =
> r570_dp_set_indexed_link_rates,
> + .sst = r570_dp_sst,
> + .vcpi = r570_dp_vcpi,
> },
> .chan = {
> .set_pushbuf = r570_disp_chan_set_pushbuf,
> diff --git
> a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/disp.h
> b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/disp.h
> index 06e972835d77..742b25a2a12d 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/disp.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/disp.h
> @@ -256,6 +256,8 @@ typedef struct NV0073_CTRL_DP_CTRL_PARAMS {
>      NvU32 eightLaneDpcdBaseAddr;
> } NV0073_CTRL_DP_CTRL_PARAMS;
>
> +#define NV0073_CTRL_CMD_DP_CONFIG_STREAM
> (0x731362U) /* finn: Evaluated from
> "(FINN_NV04_DISPLAY_COMMON_DP_INTERFACE_ID << 8) |
> NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS_MESSAGE_ID" */
> +
> typedef struct NV0073_CTRL_CMD_DP_CONFIG_STREAM_PARAMS {
>      NvU32 subDeviceInstance;
>      NvU32 head;
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
> b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
> index a9af94adf9ef..e0c10a7e3452 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
> @@ -6,6 +6,7 @@
> #ifndef __NVKM_RM_H__
> #define __NVKM_RM_H__
> #include "handles.h"
> +struct nvkm_ior;
> struct nvkm_outp;
> struct r535_gr;
>
> @@ -93,6 +94,10 @@ struct nvkm_rm_api {
> struct {
> int (*get_caps)(struct nvkm_disp *, int
> *link_bw, bool *mst, bool *wm);
> int (*set_indexed_link_rates)(struct
> nvkm_outp *);
> + int (*sst)(struct nvkm_ior *, int head, bool
> ef,
> +    u32 watermark, u32 hblanksym, u32
> vblanksym);
> + void (*vcpi)(struct nvkm_ior *, int head,
> +      u8 slot, u8 slot_nr, u16 pbn,
> u16 aligned_pbn);
> } dp;
>
> struct {