On Tue, May 06, 2025 at 11:47:31PM +0800, Jun Nie wrote:
> Some display controller support flexible CRTC and DMA, such as the display
> controllers in snapdragon SoCs. CRTC can be implemented with several mixers
> in parallel, and plane fetching can be implemented with several DMA under
> umberala of a virtual drm plane.
>
> The mixer number is decided per panel resolution and clock rate constrain
> first, which happens in CRTC side. Then plane is split per mixer number
> and configure DMA accordingly.
Here you are describing a behaviour of one particular driver as a reason
to change the framework.
>
> To support such forthcoming usage case, CRTC checking shall happen before
> checking plane. Add the checking in the drm_atomic_helper_check_modeset().
So, now drivers will get two calls to atomic_check(), one coming in
circumstances which were not expected by the drivers before. Are you
sure that this won't break anything?
>
> Signed-off-by: Jun Nie <jun....@linaro.org>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index
> 5302ab3248985d3e0a47e40fd3deb7ad0d9f775b..5bca4c9683838c38574c8cb7c0bc9d57960314fe
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -816,6 +816,25 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> return ret;
> }
>
> + for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> + const struct drm_crtc_helper_funcs *funcs;
> +
> + funcs = crtc->helper_private;
> +
> + if (!funcs || !funcs->atomic_check)
> + continue;
> +
> + ret = funcs->atomic_check(crtc, state);
> + if (ret) {
> + drm_dbg_atomic(crtc->dev,
> + "[CRTC:%d:%s] atomic driver check
> failed\n",
> + crtc->base.id, crtc->name);
> + return ret;
> + }
> + }
> +
> +
> +
Too many empty lines. But the main quesiton is: why are you calling it
before mode_valid()? According to your description a better place would
be in drm_atomic_helper_check_planes().
> ret = mode_valid(state);
> if (ret)
> return ret;
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
