Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/loongson: skipped CRTC and encoder hardware reset in lsdc_create_device() --
commit 648f0256277cc04b62e3a2eb71b3474bb1ab8054 Author: Maxime Ripard <[email protected]> drm/loongson: Switch to drm_mode_config_create_initial_state This commit replaces drm_mode_config_reset() with drm_mode_config_create_initial_state() in lsdc_create_device() during probe. > diff --git a/drivers/gpu/drm/loongson/lsdc_drv.c > b/drivers/gpu/drm/loongson/lsdc_drv.c > index 5a13df31eb902..fbb2658acce40 100644 > --- a/drivers/gpu/drm/loongson/lsdc_drv.c > +++ b/drivers/gpu/drm/loongson/lsdc_drv.c > @@ -247,7 +247,9 @@ lsdc_create_device(struct pci_dev *pdev, > if (ret) > return ERR_PTR(ret); > > - drm_mode_config_reset(ddev); > + ret = drm_mode_config_create_initial_state(ddev); > + if (ret) > + return ERR_PTR(ret); [Severity: High] Does switching from drm_mode_config_reset() to drm_mode_config_create_initial_state() in lsdc_create_device() skip required CRTC and encoder hardware initialization at probe time? Unlike drm_mode_config_reset(), drm_mode_config_create_initial_state() does not invoke crtc->helper_private->hw_reset() or encoder->funcs->reset(). In drivers/gpu/drm/loongson/lsdc_crtc.c, lsdc_crtc_hw_reset() calls lsdc_crtc0_reset() and lsdc_crtc1_reset(): static void lsdc_crtc0_reset(struct lsdc_crtc *lcrtc) { struct lsdc_device *ldev = lcrtc->ldev; lsdc_wreg32(ldev, LSDC_CRTC0_CFG_REG, CFG_RESET_N | LSDC_PF_XRGB8888); } Other runtime accesses to LSDC_CRTC0_CFG_REG and LSDC_CRTC1_CFG_REG during modeset and plane updates (such as lsdc_crtc0_enable(), lsdc_crtc0_disable(), lsdc_crtc0_set_dma_step(), and lsdc_primary0_update_fb_format()) perform read-modify-write operations that neither assert CFG_RESET_N nor clear stale firmware bits such as CFG_HW_CLONE or CFG_GAMMA_EN. In addition, the encoder reset hooks in drivers/gpu/drm/loongson/lsdc_output_7a1000.c and drivers/gpu/drm/loongson/lsdc_output_7a2000.c are the only places in the driver that program LSDC_CRTC0_DVO_CONF_REG and LSDC_CRTC1_DVO_CONF_REG: static void ls7a2000_hdmi0_encoder_reset(struct drm_encoder *encoder) { struct drm_device *ddev = encoder->dev; struct lsdc_device *ldev = to_lsdc(ddev); u32 val; val = PHY_CLOCK_POL | PHY_CLOCK_EN | PHY_DATA_EN; lsdc_wreg32(ldev, LSDC_CRTC0_DVO_CONF_REG, val); /* using software gpio emulated i2c */ val = lsdc_rreg32(ldev, LSDC_HDMI0_INTF_CTRL_REG); val &= ~HW_I2C_EN; lsdc_wreg32(ldev, LSDC_HDMI0_INTF_CTRL_REG, val); /* help the hdmi phy to get out of reset state */ lsdc_wreg32(ldev, LSDC_HDMI0_PHY_CTRL_REG, HDMI_PHY_RESET_N); mdelay(20); } On LS7A2000, skipping ls7a2000_hdmi0_encoder_reset() and ls7a2000_hdmi1_encoder_reset() at probe time also leaves HW_I2C_EN set in LSDC_HDMI0_INTF_CTRL_REG and LSDC_HDMI1_INTF_CTRL_REG and leaves the HDMI PHY in reset, breaking software GPIO-emulated I2C DDC probing in ls7a2000_hdmi0_vga_connector_detect() and ls7a2000_connector_get_modes() before any atomic enable commit runs. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=23
