Re: [PATCH v4 1/2] drm/atomic-helper: Don't allocated plane state in CRTC check

[Javier Martinez Canillas <javierm@redhat.com>]

Hello Thomas,

On 10/5/22 13:40, Thomas Zimmermann wrote:
> In drm_atomic_helper_check_crtc_state(), do not add a new plane state
> to the global state if it does not exist already. Adding a new plane
> state will result in overhead for the plane during the atomic-commit
> step.
> 
> For the test in drm_atomic_helper_check_crtc_state() to succeed, it
> is important that the CRTC has an enabled primary plane after the
> commit. Simply testing the CRTC state's plane_mask for a primary plane
> is sufficient.
> 
> Note that the helper still only tests for an attached primary plane.
> Drivers have to ensure that the plane contains valid pixel information.
> 
> v3:
> 	* test for a primary plane in plane_mask (Ville)
> v2:
> 	* remove unnecessary test for plane->crtc (Ville)
> 	* inline drm_atomic_get_next_plane_state() (Ville)
> 	* acquire plane lock before accessing plane->state (Ville)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: d6b9af1097fe ("drm/atomic-helper: Add helper drm_atomic_helper_check_crtc_state()")

This patch makes sense to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

but I've a hard time parsing the subject line. Did you mean instead:

"drm/atomic-helper: Don't allocate new plane state in CRTC check" ?

or "drm/atomic-helper: Don't add a new plane state in CRTC check" ?

In any case you can fix that while applying so no need to resend IMO.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat