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

[email protected]
Newsgroups org.freedesktop.lists.nouveau,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  .../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 {
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.