diff mbox

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

Message ID 0cd2570c9cbe20d094cee0e8e9918c3a0f6af2fa.1513178989.git-series.maxime.ripard@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Ripard Dec. 13, 2017, 3:33 p.m. UTC
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@free-electrons.com>
---
 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(-)

Comments

Neil Armstrong Dec. 13, 2017, 4:11 p.m. UTC | #1
On 13/12/2017 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@free-electrons.com>
> ---
>  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 *
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Thomas van Kleef Dec. 13, 2017, 4:23 p.m. UTC | #2
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@free-electrons.com>
> ---
>  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
Maxime Ripard Dec. 14, 2017, 10:32 a.m. UTC | #3
Hi,

On Wed, Dec 13, 2017 at 05:23:04PM +0100, Thomas van Kleef wrote:
> 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?

Yes.

> Or could be present 1 additional layer when it is attached to the
> frontend?

The frontend will still consume a backend layer. In this current
serie, we still define only two layers for the reasons exposed above,
and whatever layer using hardware scaling will use the frontend.

I have a few patches that would remove that limitation, but we need a
few things in order to do that properly.

The first one would be to add a check on the alpha component of our
layers. We basically have two rules:
  - the lowest plane shouldn't use alpha at all, because of a bug in
    the display engine that would render the pixels completely
    transparents if the alpha is set to something less than 255 on the
    lowest plane.
  - we can only have a plane with alpha as the lowest plane of each
    pipe.

This effectively means that the only scenario that works would be that
there can be only one plane can use alpha at a time, that it shouldn't
be the lowest one, and that it must be assigned to the second pipe.

I worked on that a bit quite some time ago, and the only thing I was
missing was a proper way to check for that in atomic_check, but with
the work done in this serie it shouldn't be too hard.

Then, we can enable the four layers, which is not really difficult in
itself.

> Perhaps this is not relevant for this patchset.

Yep, that's quite orthogonal.

Maxime
kernel test robot Dec. 16, 2017, 9:17 a.m. UTC | #4
Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on ]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-sun4i-Support-the-Display-Engine-frontend/20171216-122702
base:    
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   WARNING: modpost: missing MODULE_LICENSE() in arch/arm/common/bL_switcher_dummy_if.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/auxdisplay/img-ascii-lcd.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/cpufreq/mediatek-cpufreq.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-ath79.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-iop.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/accel/kxsd9-i2c.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/adc/qcom-vadc-common.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/mtk-vcodec/mtk-vcodec-common.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/soc_camera/soc_scale_crop.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/tegra-cec/tegra_cec.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/mmc/host/renesas_sdhi_core.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/mtd/nand/denali_pci.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/net/ethernet/cirrus/cs89x0.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/phy/qualcomm/phy-qcom-ufs.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/pinctrl/pxa/pinctrl-pxa2xx.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/power/reset/zx-reboot.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/soc/qcom/rmtfs_mem.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/staging/comedi/drivers/ni_atmio.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/video/fbdev/mmp/mmp_disp.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in sound/soc/codecs/snd-soc-pcm512x-spi.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in sound/soc/ux500/snd-soc-ux500-mach-mop500.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in sound/soc/ux500/snd-soc-ux500-plat-dma.o
   see include/linux/module.h for more information
>> ERROR: "sun4i_frontend_update_formats" [drivers/gpu/drm/sun4i/sun4i-backend.ko] undefined!
>> ERROR: "sun4i_frontend_update_buffer" [drivers/gpu/drm/sun4i/sun4i-backend.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

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 *