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

[lyude@redhat.com]

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@gmail.com>
> ---
>  .../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 {