Re: [PATCH RESEND 23/83] drm/loongson: Switch to drm_mode_config_create_initial_state

[sashiko-bot@kernel.org]

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 <mripard@kernel.org>

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/20261105-drm-no-more-config-reset-v1-0-1f58d1e33380@kernel.org?part=23