Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] PIOR encoders do not clear `audio.crtc` on disable, leading to stale audio state being reported. --
--- Patch [5]: [PATCH 5/6] drm/nouveau/kms/nv50-: Add nouveau_encoder->audio.crtc --- commit d00b281328e092a25d08552356c0a7fc11880073 Author: Lyude Paul <[email protected]> drm/nouveau/kms/nv50-: Add nouveau_encoder->audio.crtc This commit adds an audio.crtc pointer to track the assigned CRTC for an encoder outside of modesetting locks. It replaces the usage of nouveau_encoder->crtc in the audio ELD retrieval path. > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c > b/drivers/gpu/drm/nouveau/dispnv50/disp.c > index 63d554e97fb62..3bfc04f7ef602 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -636,7 +636,7 @@ nv50_audio_component_get_eld(struct device *kdev, int > port, int dev_id, > > nv_encoder = nouveau_encoder(encoder); > nv_connector = nv_encoder->conn; > - nv_crtc = nouveau_crtc(nv_encoder->crtc); > + nv_crtc = nv_encoder->audio.crtc; > > if (!nv_crtc || nv_encoder->outp.or.id != port || > nv_crtc->index != dev_id) > continue; [Severity: Medium] Will this cause stale audio state to be reported when PIOR encoders are disabled? Before this patch, checking the cleared crtc pointer successfully skipped the disabled encoder. However, it looks like nv50_pior_atomic_disable() only clears the old crtc pointer, but not the new audio.crtc pointer: nv50_pior_atomic_disable() { ... core->func->pior->ctrl(core, nv_encoder->outp.or.id, ctrl, NULL); nv_encoder->crtc = NULL; } Does nv_encoder->audio.crtc also need to be cleared here, or does nv50_audio_disable() need to be called in nv50_pior_atomic_disable() to prevent userspace from receiving stale ELD data for a deactivated display? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
