Re: [PATCH 1/7] nouveau/disp: add GB20x HDMI vendor infoframe writer

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added `gb202_sor_hdmi_infoframe_vsi` function is never hooked into any display structures, rendering it dead code. Consequently, GB20x chips will incorrectly fall back to the legacy `gv100_sor_hdmi_infoframe_vsi` function, writing to the removed 0x6f0100 registers.
--

--- Patch [1]: [PATCH 1/7] nouveau/disp: add GB20x HDMI vendor infoframe writer ---
commit 722f58f6d8bd3ed70d39357b7f9c41339c88e50d
Author: Mohamed Ahmed <[email protected]>

nouveau/disp: add GB20x HDMI vendor infoframe writer

This commit adds a new HDMI vendor infoframe writer specifically for GB20x 
chips. It handles the hardware change where the legacy VSI unit at 0x6f0100 
was removed and replaced by shared generic infoframe units.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/gb202.c
[ ... ]
> +void
> +gb202_sor_hdmi_infoframe_vsi(struct nvkm_ior *ior, int head, void *data, u32 size)
> +{

[Severity: High]
Is this new function ever called? It appears it is not hooked into any of the
display structures, leaving it as dead code.

Without wiring this up, won't GB20x chips still incorrectly fall back to the 
legacy function when sending an HDMI vendor infoframe? For example, 
r535_sor_hdmi in drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c 
continues to unconditionally use the legacy callback:

    .infoframe_vsi = gv100_sor_hdmi_infoframe_vsi,

Could this cause MMIO faults or fail to send the vendor infoframe when 
the driver attempts to access the removed 0x6f0100 registers on GB20x
hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
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.