Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The CRTC pixel clock is disabled before the hardware is instructed to disable the CRTC. --
commit c35af4d919e849e303afa29d897c4d1581720138 Author: Joey Lu <[email protected]> drm/verisilicon: introduce per-variant hardware ops table The DC8200 and DCUltraLite share a broadly similar register layout but differ in how the bridge, CRTC, primary plane and IRQ paths are driven. This commit introduces a vs_dc_funcs vtable so each variant can supply its own implementation without scattering conditionals across multiple files. > diff --git a/drivers/gpu/drm/verisilicon/vs_crtc.c > b/drivers/gpu/drm/verisilicon/vs_crtc.c > index 0b8a35d09cd2e..1c4aac7086694 100644 > --- a/drivers/gpu/drm/verisilicon/vs_crtc.c > +++ b/drivers/gpu/drm/verisilicon/vs_crtc.c > @@ -30,6 +53,9 @@ static void vs_crtc_atomic_disable(struct drm_crtc *crtc, > drm_crtc_vblank_off(crtc); > > clk_disable_unprepare(dc->pix_clk[output]); > + > + if (dc->funcs->crtc_disable_ex) > + dc->funcs->crtc_disable_ex(dc, output); [Severity: High] Are hardware registers written after the pixel clock is disabled here? In vs_crtc_atomic_disable(), dc->funcs->crtc_disable_ex() executes out-of-order, writing to MMIO registers after the associated pixel clock has been disabled via clk_disable_unprepare(). Writing to hardware registers after disabling the pixel clock can cause bus hangs (asynchronous external aborts) resulting in kernel panics, or silently fail to latch the register value, leaving the hardware in an inconsistent state. This sequence can be triggered by local userspace through DRM atomic mode setting ioctls when the CRTC is disabled, such as when turning off the screen or performing a mode switch. Could the hardware disable callback be moved before the clock is disabled to correctly mirror the enable sequence? > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
