Re: [PATCH 6/8] drm/sun4i: sun4i_layer: Wire in the frontend

[Thomas van Kleef]

On 13-12-17 16:33, Maxime Ripard wrote:
> Now that we have a driver, we can make use of it. This is done by
> adding a flag to our custom plane state that will trigger whether we should
> use the frontend on that particular plane or not.
> 
> The rest is just plumbing to set up the backend to not perform the DMA but
> receive its data from the frontend.
> 
> Note that we're still not making any use of the frontend itself, as no one
> is setting the flag yet.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 53 ++++++++++++++++++++++++++++-
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  3 ++-
>  drivers/gpu/drm/sun4i/sun4i_layer.c   | 27 ++++++++++++--
>  drivers/gpu/drm/sun4i/sun4i_layer.h   |  1 +-
>  4 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index e83e1fe43823..f1d19767c55d 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -26,6 +26,7 @@
>  
>  #include "sun4i_backend.h"
>  #include "sun4i_drv.h"
> +#include "sun4i_frontend.h"
>  #include "sun4i_layer.h"
>  #include "sunxi_engine.h"
>  
> @@ -203,6 +204,30 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
>  	return 0;
>  }
>  
> +int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
> +					int layer, uint32_t fmt)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = sun4i_backend_drm_format_to_layer(NULL, fmt, &val);
> +	if (ret) {
> +		DRM_DEBUG_DRIVER("Invalid format\n");
> +		return ret;
> +	}
> +
> +	regmap_update_bits(backend->engine.regs,
> +			   SUN4I_BACKEND_ATTCTL_REG0(layer),
> +			   SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN,
> +			   SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN);
> +
> +	regmap_update_bits(backend->engine.regs,
> +			   SUN4I_BACKEND_ATTCTL_REG1(layer),
> +			   SUN4I_BACKEND_ATTCTL_REG1_LAY_FBFMT, val);
> +
> +	return 0;
> +}
> +
>  int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
>  				      int layer, struct drm_plane *plane)
>  {
> @@ -330,6 +355,34 @@ static int sun4i_backend_of_get_id(struct device_node *node)
>  	return ret;
>  }
>  
> +static struct sun4i_frontend *sun4i_backend_find_frontend(struct sun4i_drv *drv,
> +							  struct device_node *node)
> +{
> +	struct device_node *port, *ep, *remote;
> +	struct sun4i_frontend *frontend;
> +
> +	port = of_graph_get_port_by_id(node, 0);
> +	if (!port)
> +		return ERR_PTR(-EINVAL);
> +
> +	for_each_available_child_of_node(port, ep) {
> +		remote = of_graph_get_remote_port_parent(ep);
> +		if (!remote)
> +			continue;
> +
> +		/* does this node match any registered engines? */
> +		list_for_each_entry(frontend, &drv->frontend_list, list) {
> +			if (remote == frontend->node) {
> +				of_node_put(remote);
> +				of_node_put(port);
> +				return frontend;
> +			}
> +		}
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
>  static const struct sunxi_engine_ops sun4i_backend_engine_ops = {
>  	.commit				= sun4i_backend_commit,
>  	.layers_init			= sun4i_layers_init,
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index ba1410fd5410..636a51521e77 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -72,6 +72,7 @@
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(x)		((x) << 15)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK	GENMASK(11, 10)
>  #define SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(x)			((x) << 10)
> +#define SUN4I_BACKEND_ATTCTL_REG0_LAY_VDOEN		BIT(1)
>  
>  #define SUN4I_BACKEND_ATTCTL_REG1(l)		(0x8a0 + (0x4 * (l)))
>  #define SUN4I_BACKEND_ATTCTL_REG1_LAY_HSCAFCT		GENMASK(15, 14)
> @@ -171,5 +172,7 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
>  				       int layer, struct drm_plane *plane);
>  int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
>  				      int layer, struct drm_plane *plane);
> +int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
> +					int layer, uint32_t in_fmt);
>  
>  #endif /* _SUN4I_BACKEND_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index c3afcf888906..3b58667a06dc 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -15,6 +15,7 @@
>  #include <drm/drmP.h>
>  
>  #include "sun4i_backend.h"
> +#include "sun4i_frontend.h"
>  #include "sun4i_layer.h"
>  #include "sunxi_engine.h"
>  
> @@ -70,21 +71,41 @@ static void sun4i_backend_layer_destroy_state(struct drm_plane *plane,
>  static void sun4i_backend_layer_atomic_disable(struct drm_plane *plane,
>  					       struct drm_plane_state *old_state)
>  {
> +	struct sun4i_layer_state *layer_state = state_to_sun4i_layer_state(old_state);
>  	struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
>  	struct sun4i_backend *backend = layer->backend;
> +	struct sun4i_frontend *frontend = backend->frontend;
>  
>  	sun4i_backend_layer_enable(backend, layer->id, false);
> +
> +	if (layer_state->uses_frontend)
> +		sun4i_frontend_exit(frontend);
>  }
>  
>  static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
>  					      struct drm_plane_state *old_state)
>  {
> +	struct sun4i_layer_state *layer_state = state_to_sun4i_layer_state(plane->state);
>  	struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
>  	struct sun4i_backend *backend = layer->backend;
> +	struct sun4i_frontend *frontend = backend->frontend;
> +
> +	if (layer_state->uses_frontend) {
> +		sun4i_frontend_init(frontend);
> +		sun4i_frontend_update_coord(frontend, plane);
> +		sun4i_frontend_update_buffer(frontend, plane);
> +		sun4i_frontend_update_formats(frontend, plane,
> +					      DRM_FORMAT_ARGB8888);
> +		sun4i_backend_update_layer_frontend(backend, layer->id,
> +						    DRM_FORMAT_ARGB8888);
> +		sun4i_backend_update_layer_coord(backend, layer->id, plane);
> +		sun4i_frontend_enable(frontend);
> +	} else {
> +		sun4i_backend_update_layer_coord(backend, layer->id, plane);
> +		sun4i_backend_update_layer_formats(backend, layer->id, plane);
> +		sun4i_backend_update_layer_buffer(backend, layer->id, plane);
> +	}
>  
> -	sun4i_backend_update_layer_coord(backend, layer->id, plane);
> -	sun4i_backend_update_layer_formats(backend, layer->id, plane);
> -	sun4i_backend_update_layer_buffer(backend, layer->id, plane);
>  	sun4i_backend_layer_enable(backend, layer->id, true);
>  }
>  
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h b/drivers/gpu/drm/sun4i/sun4i_layer.h
> index d2c19348d1b0..75b4868ba87c 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h
> @@ -24,6 +24,7 @@ struct sun4i_layer {
>  
>  struct sun4i_layer_state {
>  	struct drm_plane_state	state;
> +	bool			uses_frontend;
>  };
>  
>  static inline struct sun4i_layer *
> 

I wondered if the following is still valid?
In file sun4i_layer.c, func sun4i_layers_init

------------------
         /*
         * The hardware is a bit unusual here.
         *
         * Even though it supports 4 layers, it does the composition
         * in two separate steps.
         *
         * The first one is assigning a layer to one of its two
         * pipes. If more that 1 layer is assigned to the same pipe,
         * and if pixels overlaps, the pipe will take the pixel from
         * the layer with the highest priority.
         *
         * The second step is the actual alpha blending, that takes
         * the two pipes as input, and uses the eventual alpha
         * component to do the transparency between the two.
         *
         * This two steps scenario makes us unable to guarantee a
         * robust alpha blending between the 4 layers in all
         * situations. So we just expose two layers, one per pipe. On
         * SoCs that support it, sprites could fill the need for more
         * layers.
         */
        for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
                const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
                struct sun4i_layer *layer;

                layer = sun4i_layer_init_one(drm, backend, plane);
                if (IS_ERR(layer)) {
                        dev_err(drm->dev, "Couldn't initialize %s plane\n",
                                i ? "overlay" : "primary");
                        return ERR_CAST(layer);
                };

                DRM_DEBUG_DRIVER("Assigning %s plane to pipe %d\n",
                                 i ? "overlay" : "primary", plane->pipe);
                regmap_update_bits(engine->regs, SUN4I_BACKEND_ATTCTL_REG0(i),
                                   SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK,
                                   SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(plane->pipe));

                layer->id = i;
                planes[i] = &layer->plane;
        };
-----------------
I have been using the 3rd layer (layer2) as the input for the frontend. This essentially
overlays the other 2 layers. Is it still the case that we can only have 2 layers here?
Or could be present 1 additional layer when it is attached to the frontend?
Perhaps this is not relevant for this patchset.

Regards,

Thomas van Kleef
Vitsch Electronics
http://Vitsch.nl/
http://VitschVPN.nl/
tel: +31-(0)40-7113051
KvK nr: 17174380
BTW nr: NL142748201B01