Re: [PATCH 7/8] drm/sun4i: sun4i_layer: Add a custom atomic_check for the frontend

[Neil Armstrong]

On 13/12/2017 16:33, Maxime Ripard wrote:
> Now that we have everything in place, we can start enabling the frontend.
> This is more difficult than one would assume since there can only be one
> plane using the frontend per-backend.
> 
> We therefore need to make sure that the userspace will not try to setup
> multiple planes using it, since that would be impossible. In order to
> prevent that, we can create an atomic_check callback that will check that
> only one plane will effectively make use of the frontend in a given
> configuration, and will toggle the switch in that plane state so that the
> proper setup function can do their role.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 65 ++++++++++++++++++++++++++++-
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  2 +-
>  2 files changed, 67 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index f1d19767c55d..a7b87a12990e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> @@ -271,6 +272,69 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
>  	return 0;
>  }
>  
> +static bool sun4i_backend_plane_uses_scaler(struct drm_plane_state *state)
> +{
> +	u16 src_h = state->src_h >> 16;
> +	u16 src_w = state->src_w >> 16;
> +
> +	DRM_DEBUG_DRIVER("Input size %dx%d, output size %dx%d\n",
> +			 src_w, src_h, state->crtc_w, state->crtc_h);
> +
> +	if ((state->crtc_h != src_h) || (state->crtc_w != src_w))
> +		return true;
> +
> +	return false;
> +}
> +
> +static bool sun4i_backend_plane_uses_frontend(struct drm_plane_state *state)
> +{
> +	struct sun4i_layer *layer = plane_to_sun4i_layer(state->plane);
> +	struct sun4i_backend *backend = layer->backend;
> +
> +	if (IS_ERR(backend->frontend))
> +		return false;
> +
> +	return sun4i_backend_plane_uses_scaler(state);
> +}
> +
> +static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> +				      struct drm_crtc_state *crtc_state)
> +{
> +	struct drm_atomic_state *state = crtc_state->state;
> +	struct drm_device *drm = state->dev;
> +	struct drm_plane *plane;
> +	unsigned int num_frontend_planes = 0;
> +
> +	DRM_DEBUG_DRIVER("Starting checking our planes\n");
> +
> +	if (!crtc_state->planes_changed)
> +		return 0;
> +
> +	drm_for_each_plane_mask(plane, drm, crtc_state->plane_mask) {
> +		struct drm_plane_state *plane_state =
> +			drm_atomic_get_plane_state(state, plane);
> +		struct sun4i_layer_state *layer_state =
> +			state_to_sun4i_layer_state(plane_state);
> +
> +		if (sun4i_backend_plane_uses_frontend(plane_state)) {
> +			DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
> +					 plane->index);
> +
> +			layer_state->uses_frontend = true;
> +			num_frontend_planes++;
> +		} else {
> +			layer_state->uses_frontend = false;
> +		}
> +	}
> +
> +	if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
> +		DRM_DEBUG_DRIVER("Too many planes going through the frontend, rejecting\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int sun4i_backend_init_sat(struct device *dev) {
>  	struct sun4i_backend *backend = dev_get_drvdata(dev);
>  	int ret;
> @@ -384,6 +448,7 @@ static struct sun4i_frontend *sun4i_backend_find_frontend(struct sun4i_drv *drv,
>  }
>  
>  static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
> +	.atomic_check			= sun4i_backend_atomic_check,
>  	.commit				= sun4i_backend_commit,
>  	.layers_init			= sun4i_layers_init,
>  	.apply_color_correction		= sun4i_backend_apply_color_correction,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 636a51521e77..3b5531440fa3 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -144,6 +144,8 @@
>  #define SUN4I_BACKEND_HWCCOLORTAB_OFF		0x4c00
>  #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
>  
> +#define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
> +
>  struct sun4i_backend {
>  	struct sunxi_engine	engine;
>  	struct sun4i_frontend	*frontend;
> 

Reviewed-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>