[PATCH v4 5/7] drm/simpledrm: Preallocate format-conversion buffer in atomic_check

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann at suse.de> 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 at suse.de>
> ---
>  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 at redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat