Thomas Zimmermann <tzimmermann@xxxxxxx> writes:
Hello Thomas,
> Preallocate the format-conversion state's storage in the plane's
> atomic_check function if a format conversion is necessary. Allows
> the update to fail if no memory is available. Avoids the same
> allocation within atomic_update, which may not fail.
>
> Also inline drm_plane_helper_atomic_check() into the driver and thus
> return early for invisible planes. Avoids memory allocation entirely
> in this case.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
> drivers/gpu/drm/tiny/simpledrm.c | 41 +++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 6788f465995b3..d2280c81cbaae 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -19,6 +19,7 @@
> #include <drm/drm_drv.h>
> #include <drm/drm_fbdev_generic.h>
> #include <drm/drm_format_helper.h>
> +#include <drm/drm_framebuffer.h>
> #include <drm/drm_gem_atomic_helper.h>
> #include <drm/drm_gem_framebuffer_helper.h>
> #include <drm/drm_gem_shmem_helper.h>
> @@ -579,6 +580,44 @@ static const uint64_t simpledrm_primary_plane_format_modifiers[] = {
> DRM_FORMAT_MOD_INVALID
> };
>
> +static int simpledrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
> + struct drm_atomic_state *state)
> +{
> + struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_shadow_plane_state *new_shadow_plane_state =
> + to_drm_shadow_plane_state(new_plane_state);
> + struct drm_framebuffer *new_fb = new_plane_state->fb;
> + struct drm_crtc *new_crtc = new_plane_state->crtc;
> + struct drm_crtc_state *new_crtc_state = NULL;
> + struct drm_device *dev = plane->dev;
> + struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
> + int ret;
> +
> + if (new_crtc)
> + new_crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc);
> +
> + ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
> + DRM_PLANE_NO_SCALING,
> + DRM_PLANE_NO_SCALING,
> + false, false);
Same comment that with the ssd130x driver. I think that we should use the
drm_plane_helper_atomic_check() helper instead of open coding it in each
driver. But regardless of what's decided on that, the change looks good:
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
