Re: [PATCH v6 25/28] drm/tidss: Implement hw_reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/tidss: Missing hw_reset invocation and PM runtime API violation
--

commit 03dacf0e68abfb247fcee4bcb84fa7f48104c834
Author: Maxime Ripard <mripard@kernel.org>
drm/tidss: Implement hw_reset

This commit renames dispc_init_hw() to dispc_reset_hw() and removes its
unconditional call from dispc_init(), instead implementing a hw_reset hook
in drm_mode_config_funcs.

> diff --git a/drivers/gpu/drm/tidss/tidss_kms.c b/drivers/gpu/drm/tidss/tidss_kms.c
> --- a/drivers/gpu/drm/tidss/tidss_kms.c
> +++ b/drivers/gpu/drm/tidss/tidss_kms.c
[ ... ]
> @@ -116,10 +117,23 @@ static int tidss_atomic_check(struct drm_device *ddev,
>  	return 0;
>  }
>  
> +static void tidss_hw_reset(struct drm_device *dev)
> +{
> +	struct tidss_device *tidss = to_tidss(dev);
> +	int ret;
> +
> +	ret = dispc_reset_hw(tidss->dispc);

[Severity: High]
Will this hw_reset hook actually get called during initialization?

Looking at the driver's probe sequence in tidss_probe(), the driver calls
drm_mode_config_reset(). This function ignores the mode_config_funcs.hw_reset
hook. Because the patch removes the unconditional call to the hardware reset
function in dispc_init(), does this leave the display controller in an
unknown state without ever resetting it?

Additionally, if the hook were invoked, would pm_runtime_set_active() fail?

In the probe path, runtime PM is enabled before the DRM state is fully
initialized:

drivers/gpu/drm/tidss/tidss_drv.c:tidss_probe() {
    ...
    pm_runtime_enable(dev);
    ...
    drm_mode_config_reset(ddev);
    ...
}

If dispc_reset_hw() runs while runtime PM is enabled, its internal call to
pm_runtime_set_active(dev) will return -EAGAIN because the disable_depth is
zero. This failure would cause dispc_reset_hw() to return early before calling
dispc_softreset(). Can this bypass the actual hardware soft reset completely
even if the hook is reached?

> +	if (ret)
> +		drm_warn(dev, "Couldn't reset dispc");
> +
> +	drm_mode_config_helper_hw_reset(dev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-drm-state-readout-v6-0-6cc02cd10998@kernel.org?part=25