[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

Not all sunxi platforms with the first version of the Display Engine
support an alpha component on the plane with the lowest z position
(as in: lowest z-pos), that gets blended with the background color.

In particular, the A13 is known to have this limitation. However, it was
recently discovered that the A20 and A33 are capable of having alpha on
their lowest plane.

Thus, this introduces a specific quirk to indicate such support,
per-platform. Since this was not tested on sun4i and sun6i platforms, a
conservative approach is kept and this feature is not supported.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---

Changes since v1:
* reordered backend declaration;
* updated comment to reflect that not all platforms are affected;
* used num_alpha_planes_max variable instead of define, since it is now
  dynamic;
* reordered check to have quirk first in associated conditional.

 drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index de0a76dfa1a2..eda9466b793b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -34,6 +34,8 @@
 struct sun4i_backend_quirks {
 	/* backend <-> TCON muxing selection done in backend */
 	bool needs_output_muxing;
+	/* alpha at the lowest z position is not always supported */
+	bool supports_lowest_plane_alpha;
 };
 
 static const u32 sunxi_rgb2yuv_coef[12] = {
@@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 				      struct drm_crtc_state *crtc_state)
 {
 	struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
+	struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
 	struct drm_atomic_state *state = crtc_state->state;
 	struct drm_device *drm = state->dev;
 	struct drm_plane *plane;
 	unsigned int num_planes = 0;
 	unsigned int num_alpha_planes = 0;
 	unsigned int num_frontend_planes = 0;
+	unsigned int num_alpha_planes_max = 1;
 	unsigned int num_yuv_planes = 0;
 	unsigned int current_pipe = 0;
 	unsigned int i;
@@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 	 * 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
+	 * the two pipes as input, and uses the potential alpha
 	 * component to do the transparency between the two.
 	 *
-	 * This two steps scenario makes us unable to guarantee a
+	 * This two-step scenario makes us unable to guarantee a
 	 * robust alpha blending between the 4 layers in all
 	 * situations, since this means that we need to have one layer
 	 * with alpha at the lowest position of our two pipes.
 	 *
-	 * However, we cannot even do that, since the hardware has a
-	 * bug where the lowest plane of the lowest pipe (pipe 0,
-	 * priority 0), if it has any alpha, will discard the pixel
-	 * entirely and just display the pixels in the background
-	 * color (black by default).
+	 * However, we cannot even do that on every platform, since the
+	 * hardware has a bug where the lowest plane of the lowest pipe
+	 * (pipe 0, priority 0), if it has any alpha, will discard the
+	 * pixel data entirely and just display the pixels in the
+	 * background color (black by default).
 	 *
-	 * This means that we effectively have only three valid
-	 * configurations with alpha, all of them with the alpha being
-	 * on pipe1 with the lowest position, which can be 1, 2 or 3
-	 * depending on the number of planes and their zpos.
+	 * This means that on the affected platforms, we effectively
+	 * have only three valid configurations with alpha, all of them
+	 * with the alpha being on pipe1 with the lowest position, which
+	 * can be 1, 2 or 3 depending on the number of planes and their zpos.
 	 */
-	if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
+
+	/* For platforms that are not affected by the issue described above. */
+	if (backend->quirks->supports_lowest_plane_alpha)
+		num_alpha_planes_max++;
+
+	if (num_alpha_planes > num_alpha_planes_max) {
 		DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
 		return -EINVAL;
 	}
 
 	/* We can't have an alpha plane at the lowest position */
-	if (plane_states[0]->fb->format->has_alpha ||
-	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
+	if (!backend->quirks->supports_lowest_plane_alpha &&
+	    (plane_states[0]->fb->format->has_alpha ||
+	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
 		return -EINVAL;
 
 	for (i = 1; i < num_planes; i++) {
@@ -941,9 +951,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = {
 
 static const struct sun4i_backend_quirks sun7i_backend_quirks = {
 	.needs_output_muxing = true,
+	.supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
+	.supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun9i_backend_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 4caee0392fa4..c428952f3661 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -167,7 +167,6 @@
 #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
 
 #define SUN4I_BACKEND_NUM_LAYERS		4
-#define SUN4I_BACKEND_NUM_ALPHA_LAYERS		1
 #define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
 #define SUN4I_BACKEND_NUM_YUV_PLANES		1
 
-- 
2.18.0

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

Not all sunxi platforms with the first version of the Display Engine
support an alpha component on the plane with the lowest z position
(as in: lowest z-pos), that gets blended with the background color.

In particular, the A13 is known to have this limitation. However, it was
recently discovered that the A20 and A33 are capable of having alpha on
their lowest plane.

Thus, this introduces a specific quirk to indicate such support,
per-platform. Since this was not tested on sun4i and sun6i platforms, a
conservative approach is kept and this feature is not supported.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---

Changes since v1:
* reordered backend declaration;
* updated comment to reflect that not all platforms are affected;
* used num_alpha_planes_max variable instead of define, since it is now
  dynamic;
* reordered check to have quirk first in associated conditional.

 drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index de0a76dfa1a2..eda9466b793b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -34,6 +34,8 @@
 struct sun4i_backend_quirks {
 	/* backend <-> TCON muxing selection done in backend */
 	bool needs_output_muxing;
+	/* alpha at the lowest z position is not always supported */
+	bool supports_lowest_plane_alpha;
 };
 
 static const u32 sunxi_rgb2yuv_coef[12] = {
@@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 				      struct drm_crtc_state *crtc_state)
 {
 	struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
+	struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
 	struct drm_atomic_state *state = crtc_state->state;
 	struct drm_device *drm = state->dev;
 	struct drm_plane *plane;
 	unsigned int num_planes = 0;
 	unsigned int num_alpha_planes = 0;
 	unsigned int num_frontend_planes = 0;
+	unsigned int num_alpha_planes_max = 1;
 	unsigned int num_yuv_planes = 0;
 	unsigned int current_pipe = 0;
 	unsigned int i;
@@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 	 * 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
+	 * the two pipes as input, and uses the potential alpha
 	 * component to do the transparency between the two.
 	 *
-	 * This two steps scenario makes us unable to guarantee a
+	 * This two-step scenario makes us unable to guarantee a
 	 * robust alpha blending between the 4 layers in all
 	 * situations, since this means that we need to have one layer
 	 * with alpha at the lowest position of our two pipes.
 	 *
-	 * However, we cannot even do that, since the hardware has a
-	 * bug where the lowest plane of the lowest pipe (pipe 0,
-	 * priority 0), if it has any alpha, will discard the pixel
-	 * entirely and just display the pixels in the background
-	 * color (black by default).
+	 * However, we cannot even do that on every platform, since the
+	 * hardware has a bug where the lowest plane of the lowest pipe
+	 * (pipe 0, priority 0), if it has any alpha, will discard the
+	 * pixel data entirely and just display the pixels in the
+	 * background color (black by default).
 	 *
-	 * This means that we effectively have only three valid
-	 * configurations with alpha, all of them with the alpha being
-	 * on pipe1 with the lowest position, which can be 1, 2 or 3
-	 * depending on the number of planes and their zpos.
+	 * This means that on the affected platforms, we effectively
+	 * have only three valid configurations with alpha, all of them
+	 * with the alpha being on pipe1 with the lowest position, which
+	 * can be 1, 2 or 3 depending on the number of planes and their zpos.
 	 */
-	if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
+
+	/* For platforms that are not affected by the issue described above. */
+	if (backend->quirks->supports_lowest_plane_alpha)
+		num_alpha_planes_max++;
+
+	if (num_alpha_planes > num_alpha_planes_max) {
 		DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
 		return -EINVAL;
 	}
 
 	/* We can't have an alpha plane at the lowest position */
-	if (plane_states[0]->fb->format->has_alpha ||
-	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
+	if (!backend->quirks->supports_lowest_plane_alpha &&
+	    (plane_states[0]->fb->format->has_alpha ||
+	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
 		return -EINVAL;
 
 	for (i = 1; i < num_planes; i++) {
@@ -941,9 +951,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = {
 
 static const struct sun4i_backend_quirks sun7i_backend_quirks = {
 	.needs_output_muxing = true,
+	.supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
+	.supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun9i_backend_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 4caee0392fa4..c428952f3661 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -167,7 +167,6 @@
 #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
 
 #define SUN4I_BACKEND_NUM_LAYERS		4
-#define SUN4I_BACKEND_NUM_ALPHA_LAYERS		1
 #define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
 #define SUN4I_BACKEND_NUM_YUV_PLANES		1
 
-- 
2.18.0

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

Not all sunxi platforms with the first version of the Display Engine
support an alpha component on the plane with the lowest z position
(as in: lowest z-pos), that gets blended with the background color.

In particular, the A13 is known to have this limitation. However, it was
recently discovered that the A20 and A33 are capable of having alpha on
their lowest plane.

Thus, this introduces a specific quirk to indicate such support,
per-platform. Since this was not tested on sun4i and sun6i platforms, a
conservative approach is kept and this feature is not supported.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
---

Changes since v1:
* reordered backend declaration;
* updated comment to reflect that not all platforms are affected;
* used num_alpha_planes_max variable instead of define, since it is now
  dynamic;
* reordered check to have quirk first in associated conditional.

 drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +++++++++++++++++----------
 drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index de0a76dfa1a2..eda9466b793b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -34,6 +34,8 @@
 struct sun4i_backend_quirks {
 	/* backend <-> TCON muxing selection done in backend */
 	bool needs_output_muxing;
+	/* alpha at the lowest z position is not always supported */
+	bool supports_lowest_plane_alpha;
 };
 
 static const u32 sunxi_rgb2yuv_coef[12] = {
@@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 				      struct drm_crtc_state *crtc_state)
 {
 	struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
+	struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
 	struct drm_atomic_state *state = crtc_state->state;
 	struct drm_device *drm = state->dev;
 	struct drm_plane *plane;
 	unsigned int num_planes = 0;
 	unsigned int num_alpha_planes = 0;
 	unsigned int num_frontend_planes = 0;
+	unsigned int num_alpha_planes_max = 1;
 	unsigned int num_yuv_planes = 0;
 	unsigned int current_pipe = 0;
 	unsigned int i;
@@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
 	 * 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
+	 * the two pipes as input, and uses the potential alpha
 	 * component to do the transparency between the two.
 	 *
-	 * This two steps scenario makes us unable to guarantee a
+	 * This two-step scenario makes us unable to guarantee a
 	 * robust alpha blending between the 4 layers in all
 	 * situations, since this means that we need to have one layer
 	 * with alpha at the lowest position of our two pipes.
 	 *
-	 * However, we cannot even do that, since the hardware has a
-	 * bug where the lowest plane of the lowest pipe (pipe 0,
-	 * priority 0), if it has any alpha, will discard the pixel
-	 * entirely and just display the pixels in the background
-	 * color (black by default).
+	 * However, we cannot even do that on every platform, since the
+	 * hardware has a bug where the lowest plane of the lowest pipe
+	 * (pipe 0, priority 0), if it has any alpha, will discard the
+	 * pixel data entirely and just display the pixels in the
+	 * background color (black by default).
 	 *
-	 * This means that we effectively have only three valid
-	 * configurations with alpha, all of them with the alpha being
-	 * on pipe1 with the lowest position, which can be 1, 2 or 3
-	 * depending on the number of planes and their zpos.
+	 * This means that on the affected platforms, we effectively
+	 * have only three valid configurations with alpha, all of them
+	 * with the alpha being on pipe1 with the lowest position, which
+	 * can be 1, 2 or 3 depending on the number of planes and their zpos.
 	 */
-	if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
+
+	/* For platforms that are not affected by the issue described above. */
+	if (backend->quirks->supports_lowest_plane_alpha)
+		num_alpha_planes_max++;
+
+	if (num_alpha_planes > num_alpha_planes_max) {
 		DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
 		return -EINVAL;
 	}
 
 	/* We can't have an alpha plane at the lowest position */
-	if (plane_states[0]->fb->format->has_alpha ||
-	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
+	if (!backend->quirks->supports_lowest_plane_alpha &&
+	    (plane_states[0]->fb->format->has_alpha ||
+	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
 		return -EINVAL;
 
 	for (i = 1; i < num_planes; i++) {
@@ -941,9 +951,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = {
 
 static const struct sun4i_backend_quirks sun7i_backend_quirks = {
 	.needs_output_muxing = true,
+	.supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
+	.supports_lowest_plane_alpha = true,
 };
 
 static const struct sun4i_backend_quirks sun9i_backend_quirks = {
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 4caee0392fa4..c428952f3661 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -167,7 +167,6 @@
 #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
 
 #define SUN4I_BACKEND_NUM_LAYERS		4
-#define SUN4I_BACKEND_NUM_ALPHA_LAYERS		1
 #define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
 #define SUN4I_BACKEND_NUM_YUV_PLANES		1
 
-- 
2.18.0

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 6382 bytes --]

On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> Not all sunxi platforms with the first version of the Display Engine
> support an alpha component on the plane with the lowest z position
> (as in: lowest z-pos), that gets blended with the background color.
> 
> In particular, the A13 is known to have this limitation. However, it was
> recently discovered that the A20 and A33 are capable of having alpha on
> their lowest plane.
> 
> Thus, this introduces a specific quirk to indicate such support,
> per-platform. Since this was not tested on sun4i and sun6i platforms, a
> conservative approach is kept and this feature is not supported.

Woops, I forgot to include PATCH 1/2 from v1, which did not change.

Would it be preferable for me to send v2 again (and maybe call it v3)
for convenience?

Sorry about that,

Paul

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> 
> Changes since v1:
> * reordered backend declaration;
> * updated comment to reflect that not all platforms are affected;
> * used num_alpha_planes_max variable instead of define, since it is now
>   dynamic;
> * reordered check to have quirk first in associated conditional.
> 
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +++++++++++++++++----------
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index de0a76dfa1a2..eda9466b793b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -34,6 +34,8 @@
>  struct sun4i_backend_quirks {
>  	/* backend <-> TCON muxing selection done in backend */
>  	bool needs_output_muxing;
> +	/* alpha at the lowest z position is not always supported */
> +	bool supports_lowest_plane_alpha;
>  };
>  
>  static const u32 sunxi_rgb2yuv_coef[12] = {
> @@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
>  				      struct drm_crtc_state *crtc_state)
>  {
>  	struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
> +	struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
>  	struct drm_atomic_state *state = crtc_state->state;
>  	struct drm_device *drm = state->dev;
>  	struct drm_plane *plane;
>  	unsigned int num_planes = 0;
>  	unsigned int num_alpha_planes = 0;
>  	unsigned int num_frontend_planes = 0;
> +	unsigned int num_alpha_planes_max = 1;
>  	unsigned int num_yuv_planes = 0;
>  	unsigned int current_pipe = 0;
>  	unsigned int i;
> @@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
>  	 * 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
> +	 * the two pipes as input, and uses the potential alpha
>  	 * component to do the transparency between the two.
>  	 *
> -	 * This two steps scenario makes us unable to guarantee a
> +	 * This two-step scenario makes us unable to guarantee a
>  	 * robust alpha blending between the 4 layers in all
>  	 * situations, since this means that we need to have one layer
>  	 * with alpha at the lowest position of our two pipes.
>  	 *
> -	 * However, we cannot even do that, since the hardware has a
> -	 * bug where the lowest plane of the lowest pipe (pipe 0,
> -	 * priority 0), if it has any alpha, will discard the pixel
> -	 * entirely and just display the pixels in the background
> -	 * color (black by default).
> +	 * However, we cannot even do that on every platform, since the
> +	 * hardware has a bug where the lowest plane of the lowest pipe
> +	 * (pipe 0, priority 0), if it has any alpha, will discard the
> +	 * pixel data entirely and just display the pixels in the
> +	 * background color (black by default).
>  	 *
> -	 * This means that we effectively have only three valid
> -	 * configurations with alpha, all of them with the alpha being
> -	 * on pipe1 with the lowest position, which can be 1, 2 or 3
> -	 * depending on the number of planes and their zpos.
> +	 * This means that on the affected platforms, we effectively
> +	 * have only three valid configurations with alpha, all of them
> +	 * with the alpha being on pipe1 with the lowest position, which
> +	 * can be 1, 2 or 3 depending on the number of planes and their zpos.
>  	 */
> -	if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
> +
> +	/* For platforms that are not affected by the issue described above. */
> +	if (backend->quirks->supports_lowest_plane_alpha)
> +		num_alpha_planes_max++;
> +
> +	if (num_alpha_planes > num_alpha_planes_max) {
>  		DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
>  		return -EINVAL;
>  	}
>  
>  	/* We can't have an alpha plane at the lowest position */
> -	if (plane_states[0]->fb->format->has_alpha ||
> -	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
> +	if (!backend->quirks->supports_lowest_plane_alpha &&
> +	    (plane_states[0]->fb->format->has_alpha ||
> +	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
>  		return -EINVAL;
>  
>  	for (i = 1; i < num_planes; i++) {
> @@ -941,9 +951,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = {
>  
>  static const struct sun4i_backend_quirks sun7i_backend_quirks = {
>  	.needs_output_muxing = true,
> +	.supports_lowest_plane_alpha = true,
>  };
>  
>  static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
> +	.supports_lowest_plane_alpha = true,
>  };
>  
>  static const struct sun4i_backend_quirks sun9i_backend_quirks = {
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 4caee0392fa4..c428952f3661 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -167,7 +167,6 @@
>  #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
>  
>  #define SUN4I_BACKEND_NUM_LAYERS		4
> -#define SUN4I_BACKEND_NUM_ALPHA_LAYERS		1
>  #define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
>  #define SUN4I_BACKEND_NUM_YUV_PLANES		1
>  
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> Not all sunxi platforms with the first version of the Display Engine
> support an alpha component on the plane with the lowest z position
> (as in: lowest z-pos), that gets blended with the background color.
> 
> In particular, the A13 is known to have this limitation. However, it was
> recently discovered that the A20 and A33 are capable of having alpha on
> their lowest plane.
> 
> Thus, this introduces a specific quirk to indicate such support,
> per-platform. Since this was not tested on sun4i and sun6i platforms, a
> conservative approach is kept and this feature is not supported.

Woops, I forgot to include PATCH 1/2 from v1, which did not change.

Would it be preferable for me to send v2 again (and maybe call it v3)
for convenience?

Sorry about that,

Paul

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
> 
> Changes since v1:
> * reordered backend declaration;
> * updated comment to reflect that not all platforms are affected;
> * used num_alpha_planes_max variable instead of define, since it is now
>   dynamic;
> * reordered check to have quirk first in associated conditional.
> 
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +++++++++++++++++----------
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index de0a76dfa1a2..eda9466b793b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -34,6 +34,8 @@
>  struct sun4i_backend_quirks {
>  	/* backend <-> TCON muxing selection done in backend */
>  	bool needs_output_muxing;
> +	/* alpha at the lowest z position is not always supported */
> +	bool supports_lowest_plane_alpha;
>  };
>  
>  static const u32 sunxi_rgb2yuv_coef[12] = {
> @@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
>  				      struct drm_crtc_state *crtc_state)
>  {
>  	struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
> +	struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
>  	struct drm_atomic_state *state = crtc_state->state;
>  	struct drm_device *drm = state->dev;
>  	struct drm_plane *plane;
>  	unsigned int num_planes = 0;
>  	unsigned int num_alpha_planes = 0;
>  	unsigned int num_frontend_planes = 0;
> +	unsigned int num_alpha_planes_max = 1;
>  	unsigned int num_yuv_planes = 0;
>  	unsigned int current_pipe = 0;
>  	unsigned int i;
> @@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
>  	 * 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
> +	 * the two pipes as input, and uses the potential alpha
>  	 * component to do the transparency between the two.
>  	 *
> -	 * This two steps scenario makes us unable to guarantee a
> +	 * This two-step scenario makes us unable to guarantee a
>  	 * robust alpha blending between the 4 layers in all
>  	 * situations, since this means that we need to have one layer
>  	 * with alpha at the lowest position of our two pipes.
>  	 *
> -	 * However, we cannot even do that, since the hardware has a
> -	 * bug where the lowest plane of the lowest pipe (pipe 0,
> -	 * priority 0), if it has any alpha, will discard the pixel
> -	 * entirely and just display the pixels in the background
> -	 * color (black by default).
> +	 * However, we cannot even do that on every platform, since the
> +	 * hardware has a bug where the lowest plane of the lowest pipe
> +	 * (pipe 0, priority 0), if it has any alpha, will discard the
> +	 * pixel data entirely and just display the pixels in the
> +	 * background color (black by default).
>  	 *
> -	 * This means that we effectively have only three valid
> -	 * configurations with alpha, all of them with the alpha being
> -	 * on pipe1 with the lowest position, which can be 1, 2 or 3
> -	 * depending on the number of planes and their zpos.
> +	 * This means that on the affected platforms, we effectively
> +	 * have only three valid configurations with alpha, all of them
> +	 * with the alpha being on pipe1 with the lowest position, which
> +	 * can be 1, 2 or 3 depending on the number of planes and their zpos.
>  	 */
> -	if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
> +
> +	/* For platforms that are not affected by the issue described above. */
> +	if (backend->quirks->supports_lowest_plane_alpha)
> +		num_alpha_planes_max++;
> +
> +	if (num_alpha_planes > num_alpha_planes_max) {
>  		DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
>  		return -EINVAL;
>  	}
>  
>  	/* We can't have an alpha plane at the lowest position */
> -	if (plane_states[0]->fb->format->has_alpha ||
> -	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
> +	if (!backend->quirks->supports_lowest_plane_alpha &&
> +	    (plane_states[0]->fb->format->has_alpha ||
> +	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
>  		return -EINVAL;
>  
>  	for (i = 1; i < num_planes; i++) {
> @@ -941,9 +951,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = {
>  
>  static const struct sun4i_backend_quirks sun7i_backend_quirks = {
>  	.needs_output_muxing = true,
> +	.supports_lowest_plane_alpha = true,
>  };
>  
>  static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
> +	.supports_lowest_plane_alpha = true,
>  };
>  
>  static const struct sun4i_backend_quirks sun9i_backend_quirks = {
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 4caee0392fa4..c428952f3661 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -167,7 +167,6 @@
>  #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
>  
>  #define SUN4I_BACKEND_NUM_LAYERS		4
> -#define SUN4I_BACKEND_NUM_ALPHA_LAYERS		1
>  #define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
>  #define SUN4I_BACKEND_NUM_YUV_PLANES		1
>  
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180718/c540a412/attachment.sig>

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 6263 bytes --]

On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> Not all sunxi platforms with the first version of the Display Engine
> support an alpha component on the plane with the lowest z position
> (as in: lowest z-pos), that gets blended with the background color.
> 
> In particular, the A13 is known to have this limitation. However, it was
> recently discovered that the A20 and A33 are capable of having alpha on
> their lowest plane.
> 
> Thus, this introduces a specific quirk to indicate such support,
> per-platform. Since this was not tested on sun4i and sun6i platforms, a
> conservative approach is kept and this feature is not supported.

Woops, I forgot to include PATCH 1/2 from v1, which did not change.

Would it be preferable for me to send v2 again (and maybe call it v3)
for convenience?

Sorry about that,

Paul

> Signed-off-by: Paul Kocialkowski <paul.kocialkowski-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> Changes since v1:
> * reordered backend declaration;
> * updated comment to reflect that not all platforms are affected;
> * used num_alpha_planes_max variable instead of define, since it is now
>   dynamic;
> * reordered check to have quirk first in associated conditional.
> 
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 40 +++++++++++++++++----------
>  drivers/gpu/drm/sun4i/sun4i_backend.h |  1 -
>  2 files changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index de0a76dfa1a2..eda9466b793b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -34,6 +34,8 @@
>  struct sun4i_backend_quirks {
>  	/* backend <-> TCON muxing selection done in backend */
>  	bool needs_output_muxing;
> +	/* alpha at the lowest z position is not always supported */
> +	bool supports_lowest_plane_alpha;
>  };
>  
>  static const u32 sunxi_rgb2yuv_coef[12] = {
> @@ -463,12 +465,14 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
>  				      struct drm_crtc_state *crtc_state)
>  {
>  	struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
> +	struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
>  	struct drm_atomic_state *state = crtc_state->state;
>  	struct drm_device *drm = state->dev;
>  	struct drm_plane *plane;
>  	unsigned int num_planes = 0;
>  	unsigned int num_alpha_planes = 0;
>  	unsigned int num_frontend_planes = 0;
> +	unsigned int num_alpha_planes_max = 1;
>  	unsigned int num_yuv_planes = 0;
>  	unsigned int current_pipe = 0;
>  	unsigned int i;
> @@ -532,33 +536,39 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
>  	 * 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
> +	 * the two pipes as input, and uses the potential alpha
>  	 * component to do the transparency between the two.
>  	 *
> -	 * This two steps scenario makes us unable to guarantee a
> +	 * This two-step scenario makes us unable to guarantee a
>  	 * robust alpha blending between the 4 layers in all
>  	 * situations, since this means that we need to have one layer
>  	 * with alpha at the lowest position of our two pipes.
>  	 *
> -	 * However, we cannot even do that, since the hardware has a
> -	 * bug where the lowest plane of the lowest pipe (pipe 0,
> -	 * priority 0), if it has any alpha, will discard the pixel
> -	 * entirely and just display the pixels in the background
> -	 * color (black by default).
> +	 * However, we cannot even do that on every platform, since the
> +	 * hardware has a bug where the lowest plane of the lowest pipe
> +	 * (pipe 0, priority 0), if it has any alpha, will discard the
> +	 * pixel data entirely and just display the pixels in the
> +	 * background color (black by default).
>  	 *
> -	 * This means that we effectively have only three valid
> -	 * configurations with alpha, all of them with the alpha being
> -	 * on pipe1 with the lowest position, which can be 1, 2 or 3
> -	 * depending on the number of planes and their zpos.
> +	 * This means that on the affected platforms, we effectively
> +	 * have only three valid configurations with alpha, all of them
> +	 * with the alpha being on pipe1 with the lowest position, which
> +	 * can be 1, 2 or 3 depending on the number of planes and their zpos.
>  	 */
> -	if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
> +
> +	/* For platforms that are not affected by the issue described above. */
> +	if (backend->quirks->supports_lowest_plane_alpha)
> +		num_alpha_planes_max++;
> +
> +	if (num_alpha_planes > num_alpha_planes_max) {
>  		DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
>  		return -EINVAL;
>  	}
>  
>  	/* We can't have an alpha plane at the lowest position */
> -	if (plane_states[0]->fb->format->has_alpha ||
> -	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
> +	if (!backend->quirks->supports_lowest_plane_alpha &&
> +	    (plane_states[0]->fb->format->has_alpha ||
> +	    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
>  		return -EINVAL;
>  
>  	for (i = 1; i < num_planes; i++) {
> @@ -941,9 +951,11 @@ static const struct sun4i_backend_quirks sun6i_backend_quirks = {
>  
>  static const struct sun4i_backend_quirks sun7i_backend_quirks = {
>  	.needs_output_muxing = true,
> +	.supports_lowest_plane_alpha = true,
>  };
>  
>  static const struct sun4i_backend_quirks sun8i_a33_backend_quirks = {
> +	.supports_lowest_plane_alpha = true,
>  };
>  
>  static const struct sun4i_backend_quirks sun9i_backend_quirks = {
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 4caee0392fa4..c428952f3661 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -167,7 +167,6 @@
>  #define SUN4I_BACKEND_PIPE_OFF(p)		(0x5000 + (0x400 * (p)))
>  
>  #define SUN4I_BACKEND_NUM_LAYERS		4
> -#define SUN4I_BACKEND_NUM_ALPHA_LAYERS		1
>  #define SUN4I_BACKEND_NUM_FRONTEND_LAYERS	1
>  #define SUN4I_BACKEND_NUM_YUV_PLANES		1
>  
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On Wed, Jul 18, 2018 at 11:35:23AM +0200, Paul Kocialkowski wrote:
> On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> > Not all sunxi platforms with the first version of the Display Engine
> > support an alpha component on the plane with the lowest z position
> > (as in: lowest z-pos), that gets blended with the background color.
> > 
> > In particular, the A13 is known to have this limitation. However, it was
> > recently discovered that the A20 and A33 are capable of having alpha on
> > their lowest plane.
> > 
> > Thus, this introduces a specific quirk to indicate such support,
> > per-platform. Since this was not tested on sun4i and sun6i platforms, a
> > conservative approach is kept and this feature is not supported.
> 
> Woops, I forgot to include PATCH 1/2 from v1, which did not change.
> 
> Would it be preferable for me to send v2 again (and maybe call it v3)
> for convenience?

Yeah, resend it, and call it v2 resend (explaining why a resend was
needed).

Maxime


-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Maxime Ripard]

On Wed, Jul 18, 2018 at 11:35:23AM +0200, Paul Kocialkowski wrote:
> On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> > Not all sunxi platforms with the first version of the Display Engine
> > support an alpha component on the plane with the lowest z position
> > (as in: lowest z-pos), that gets blended with the background color.
> > 
> > In particular, the A13 is known to have this limitation. However, it was
> > recently discovered that the A20 and A33 are capable of having alpha on
> > their lowest plane.
> > 
> > Thus, this introduces a specific quirk to indicate such support,
> > per-platform. Since this was not tested on sun4i and sun6i platforms, a
> > conservative approach is kept and this feature is not supported.
> 
> Woops, I forgot to include PATCH 1/2 from v1, which did not change.
> 
> Would it be preferable for me to send v2 again (and maybe call it v3)
> for convenience?

Yeah, resend it, and call it v2 resend (explaining why a resend was
needed).

Maxime


-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180718/bfd9b33d/attachment.sig>

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

On Wed, Jul 18, 2018 at 11:35:23AM +0200, Paul Kocialkowski wrote:
> On Wed, 2018-07-18 at 11:29 +0200, Paul Kocialkowski wrote:
> > Not all sunxi platforms with the first version of the Display Engine
> > support an alpha component on the plane with the lowest z position
> > (as in: lowest z-pos), that gets blended with the background color.
> > 
> > In particular, the A13 is known to have this limitation. However, it was
> > recently discovered that the A20 and A33 are capable of having alpha on
> > their lowest plane.
> > 
> > Thus, this introduces a specific quirk to indicate such support,
> > per-platform. Since this was not tested on sun4i and sun6i platforms, a
> > conservative approach is kept and this feature is not supported.
> 
> Woops, I forgot to include PATCH 1/2 from v1, which did not change.
> 
> Would it be preferable for me to send v2 again (and maybe call it v3)
> for convenience?

Yeah, resend it, and call it v2 resend (explaining why a resend was
needed).

Maxime


-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7288 bytes --]

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Kocialkowski/drm-sun4i-sun4i-Introduce-a-quirk-for-lowest-plane-alpha-support/20180719-030611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
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
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun4i_backend.c: In function 'sun4i_backend_atomic_check':
>> drivers/gpu/drm/sun4i/sun4i_backend.c:560:13: error: 'struct sun4i_backend' has no member named 'quirks'
     if (backend->quirks->supports_lowest_plane_alpha)
                ^~
   drivers/gpu/drm/sun4i/sun4i_backend.c:569:14: error: 'struct sun4i_backend' has no member named 'quirks'
     if (!backend->quirks->supports_lowest_plane_alpha &&
                 ^~

vim +560 drivers/gpu/drm/sun4i/sun4i_backend.c

   463	
   464	static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
   465					      struct drm_crtc_state *crtc_state)
   466	{
   467		struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
   468		struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
   469		struct drm_atomic_state *state = crtc_state->state;
   470		struct drm_device *drm = state->dev;
   471		struct drm_plane *plane;
   472		unsigned int num_planes = 0;
   473		unsigned int num_alpha_planes = 0;
   474		unsigned int num_frontend_planes = 0;
   475		unsigned int num_alpha_planes_max = 1;
   476		unsigned int num_yuv_planes = 0;
   477		unsigned int current_pipe = 0;
   478		unsigned int i;
   479	
   480		DRM_DEBUG_DRIVER("Starting checking our planes\n");
   481	
   482		if (!crtc_state->planes_changed)
   483			return 0;
   484	
   485		drm_for_each_plane_mask(plane, drm, crtc_state->plane_mask) {
   486			struct drm_plane_state *plane_state =
   487				drm_atomic_get_plane_state(state, plane);
   488			struct sun4i_layer_state *layer_state =
   489				state_to_sun4i_layer_state(plane_state);
   490			struct drm_framebuffer *fb = plane_state->fb;
   491			struct drm_format_name_buf format_name;
   492	
   493			if (sun4i_backend_plane_uses_frontend(plane_state)) {
   494				DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
   495						 plane->index);
   496	
   497				layer_state->uses_frontend = true;
   498				num_frontend_planes++;
   499			} else {
   500				layer_state->uses_frontend = false;
   501			}
   502	
   503			DRM_DEBUG_DRIVER("Plane FB format is %s\n",
   504					 drm_get_format_name(fb->format->format,
   505							     &format_name));
   506			if (fb->format->has_alpha || (plane_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
   507				num_alpha_planes++;
   508	
   509			if (sun4i_backend_format_is_yuv(fb->format->format)) {
   510				DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
   511				num_yuv_planes++;
   512			}
   513	
   514			DRM_DEBUG_DRIVER("Plane zpos is %d\n",
   515					 plane_state->normalized_zpos);
   516	
   517			/* Sort our planes by Zpos */
   518			plane_states[plane_state->normalized_zpos] = plane_state;
   519	
   520			num_planes++;
   521		}
   522	
   523		/* All our planes were disabled, bail out */
   524		if (!num_planes)
   525			return 0;
   526	
   527		/*
   528		 * The hardware is a bit unusual here.
   529		 *
   530		 * Even though it supports 4 layers, it does the composition
   531		 * in two separate steps.
   532		 *
   533		 * The first one is assigning a layer to one of its two
   534		 * pipes. If more that 1 layer is assigned to the same pipe,
   535		 * and if pixels overlaps, the pipe will take the pixel from
   536		 * the layer with the highest priority.
   537		 *
   538		 * The second step is the actual alpha blending, that takes
   539		 * the two pipes as input, and uses the potential alpha
   540		 * component to do the transparency between the two.
   541		 *
   542		 * This two-step scenario makes us unable to guarantee a
   543		 * robust alpha blending between the 4 layers in all
   544		 * situations, since this means that we need to have one layer
   545		 * with alpha at the lowest position of our two pipes.
   546		 *
   547		 * However, we cannot even do that on every platform, since the
   548		 * hardware has a bug where the lowest plane of the lowest pipe
   549		 * (pipe 0, priority 0), if it has any alpha, will discard the
   550		 * pixel data entirely and just display the pixels in the
   551		 * background color (black by default).
   552		 *
   553		 * This means that on the affected platforms, we effectively
   554		 * have only three valid configurations with alpha, all of them
   555		 * with the alpha being on pipe1 with the lowest position, which
   556		 * can be 1, 2 or 3 depending on the number of planes and their zpos.
   557		 */
   558	
   559		/* For platforms that are not affected by the issue described above. */
 > 560		if (backend->quirks->supports_lowest_plane_alpha)
   561			num_alpha_planes_max++;
   562	
   563		if (num_alpha_planes > num_alpha_planes_max) {
   564			DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
   565			return -EINVAL;
   566		}
   567	
   568		/* We can't have an alpha plane at the lowest position */
   569		if (!backend->quirks->supports_lowest_plane_alpha &&
   570		    (plane_states[0]->fb->format->has_alpha ||
   571		    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
   572			return -EINVAL;
   573	
   574		for (i = 1; i < num_planes; i++) {
   575			struct drm_plane_state *p_state = plane_states[i];
   576			struct drm_framebuffer *fb = p_state->fb;
   577			struct sun4i_layer_state *s_state = state_to_sun4i_layer_state(p_state);
   578	
   579			/*
   580			 * The only alpha position is the lowest plane of the
   581			 * second pipe.
   582			 */
   583			if (fb->format->has_alpha || (p_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
   584				current_pipe++;
   585	
   586			s_state->pipe = current_pipe;
   587		}
   588	
   589		/* We can only have a single YUV plane at a time */
   590		if (num_yuv_planes > SUN4I_BACKEND_NUM_YUV_PLANES) {
   591			DRM_DEBUG_DRIVER("Too many planes with YUV, rejecting...\n");
   592			return -EINVAL;
   593		}
   594	
   595		if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
   596			DRM_DEBUG_DRIVER("Too many planes going through the frontend, rejecting\n");
   597			return -EINVAL;
   598		}
   599	
   600		DRM_DEBUG_DRIVER("State valid with %u planes, %u alpha, %u video, %u YUV\n",
   601				 num_planes, num_alpha_planes, num_frontend_planes,
   602				 num_yuv_planes);
   603	
   604		return 0;
   605	}
   606	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66067 bytes --]

[PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[kbuild test robot]

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Kocialkowski/drm-sun4i-sun4i-Introduce-a-quirk-for-lowest-plane-alpha-support/20180719-030611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
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
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun4i_backend.c: In function 'sun4i_backend_atomic_check':
>> drivers/gpu/drm/sun4i/sun4i_backend.c:560:13: error: 'struct sun4i_backend' has no member named 'quirks'
     if (backend->quirks->supports_lowest_plane_alpha)
                ^~
   drivers/gpu/drm/sun4i/sun4i_backend.c:569:14: error: 'struct sun4i_backend' has no member named 'quirks'
     if (!backend->quirks->supports_lowest_plane_alpha &&
                 ^~

vim +560 drivers/gpu/drm/sun4i/sun4i_backend.c

   463	
   464	static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
   465					      struct drm_crtc_state *crtc_state)
   466	{
   467		struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
   468		struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
   469		struct drm_atomic_state *state = crtc_state->state;
   470		struct drm_device *drm = state->dev;
   471		struct drm_plane *plane;
   472		unsigned int num_planes = 0;
   473		unsigned int num_alpha_planes = 0;
   474		unsigned int num_frontend_planes = 0;
   475		unsigned int num_alpha_planes_max = 1;
   476		unsigned int num_yuv_planes = 0;
   477		unsigned int current_pipe = 0;
   478		unsigned int i;
   479	
   480		DRM_DEBUG_DRIVER("Starting checking our planes\n");
   481	
   482		if (!crtc_state->planes_changed)
   483			return 0;
   484	
   485		drm_for_each_plane_mask(plane, drm, crtc_state->plane_mask) {
   486			struct drm_plane_state *plane_state =
   487				drm_atomic_get_plane_state(state, plane);
   488			struct sun4i_layer_state *layer_state =
   489				state_to_sun4i_layer_state(plane_state);
   490			struct drm_framebuffer *fb = plane_state->fb;
   491			struct drm_format_name_buf format_name;
   492	
   493			if (sun4i_backend_plane_uses_frontend(plane_state)) {
   494				DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
   495						 plane->index);
   496	
   497				layer_state->uses_frontend = true;
   498				num_frontend_planes++;
   499			} else {
   500				layer_state->uses_frontend = false;
   501			}
   502	
   503			DRM_DEBUG_DRIVER("Plane FB format is %s\n",
   504					 drm_get_format_name(fb->format->format,
   505							     &format_name));
   506			if (fb->format->has_alpha || (plane_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
   507				num_alpha_planes++;
   508	
   509			if (sun4i_backend_format_is_yuv(fb->format->format)) {
   510				DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
   511				num_yuv_planes++;
   512			}
   513	
   514			DRM_DEBUG_DRIVER("Plane zpos is %d\n",
   515					 plane_state->normalized_zpos);
   516	
   517			/* Sort our planes by Zpos */
   518			plane_states[plane_state->normalized_zpos] = plane_state;
   519	
   520			num_planes++;
   521		}
   522	
   523		/* All our planes were disabled, bail out */
   524		if (!num_planes)
   525			return 0;
   526	
   527		/*
   528		 * The hardware is a bit unusual here.
   529		 *
   530		 * Even though it supports 4 layers, it does the composition
   531		 * in two separate steps.
   532		 *
   533		 * The first one is assigning a layer to one of its two
   534		 * pipes. If more that 1 layer is assigned to the same pipe,
   535		 * and if pixels overlaps, the pipe will take the pixel from
   536		 * the layer with the highest priority.
   537		 *
   538		 * The second step is the actual alpha blending, that takes
   539		 * the two pipes as input, and uses the potential alpha
   540		 * component to do the transparency between the two.
   541		 *
   542		 * This two-step scenario makes us unable to guarantee a
   543		 * robust alpha blending between the 4 layers in all
   544		 * situations, since this means that we need to have one layer
   545		 * with alpha at the lowest position of our two pipes.
   546		 *
   547		 * However, we cannot even do that on every platform, since the
   548		 * hardware has a bug where the lowest plane of the lowest pipe
   549		 * (pipe 0, priority 0), if it has any alpha, will discard the
   550		 * pixel data entirely and just display the pixels in the
   551		 * background color (black by default).
   552		 *
   553		 * This means that on the affected platforms, we effectively
   554		 * have only three valid configurations with alpha, all of them
   555		 * with the alpha being on pipe1 with the lowest position, which
   556		 * can be 1, 2 or 3 depending on the number of planes and their zpos.
   557		 */
   558	
   559		/* For platforms that are not affected by the issue described above. */
 > 560		if (backend->quirks->supports_lowest_plane_alpha)
   561			num_alpha_planes_max++;
   562	
   563		if (num_alpha_planes > num_alpha_planes_max) {
   564			DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
   565			return -EINVAL;
   566		}
   567	
   568		/* We can't have an alpha plane at the lowest position */
   569		if (!backend->quirks->supports_lowest_plane_alpha &&
   570		    (plane_states[0]->fb->format->has_alpha ||
   571		    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
   572			return -EINVAL;
   573	
   574		for (i = 1; i < num_planes; i++) {
   575			struct drm_plane_state *p_state = plane_states[i];
   576			struct drm_framebuffer *fb = p_state->fb;
   577			struct sun4i_layer_state *s_state = state_to_sun4i_layer_state(p_state);
   578	
   579			/*
   580			 * The only alpha position is the lowest plane of the
   581			 * second pipe.
   582			 */
   583			if (fb->format->has_alpha || (p_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
   584				current_pipe++;
   585	
   586			s_state->pipe = current_pipe;
   587		}
   588	
   589		/* We can only have a single YUV plane at a time */
   590		if (num_yuv_planes > SUN4I_BACKEND_NUM_YUV_PLANES) {
   591			DRM_DEBUG_DRIVER("Too many planes with YUV, rejecting...\n");
   592			return -EINVAL;
   593		}
   594	
   595		if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
   596			DRM_DEBUG_DRIVER("Too many planes going through the frontend, rejecting\n");
   597			return -EINVAL;
   598		}
   599	
   600		DRM_DEBUG_DRIVER("State valid with %u planes, %u alpha, %u video, %u YUV\n",
   601				 num_planes, num_alpha_planes, num_frontend_planes,
   602				 num_yuv_planes);
   603	
   604		return 0;
   605	}
   606	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 66067 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180719/81deaa7e/attachment-0001.gz>

Re: [PATCH v2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7288 bytes --]

Hi Paul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on sunxi/sunxi/for-next]
[also build test ERROR on v4.18-rc5 next-20180718]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Kocialkowski/drm-sun4i-sun4i-Introduce-a-quirk-for-lowest-plane-alpha-support/20180719-030611
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git sunxi/for-next
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
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/sun4i/sun4i_backend.c: In function 'sun4i_backend_atomic_check':
>> drivers/gpu/drm/sun4i/sun4i_backend.c:560:13: error: 'struct sun4i_backend' has no member named 'quirks'
     if (backend->quirks->supports_lowest_plane_alpha)
                ^~
   drivers/gpu/drm/sun4i/sun4i_backend.c:569:14: error: 'struct sun4i_backend' has no member named 'quirks'
     if (!backend->quirks->supports_lowest_plane_alpha &&
                 ^~

vim +560 drivers/gpu/drm/sun4i/sun4i_backend.c

   463	
   464	static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
   465					      struct drm_crtc_state *crtc_state)
   466	{
   467		struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };
   468		struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
   469		struct drm_atomic_state *state = crtc_state->state;
   470		struct drm_device *drm = state->dev;
   471		struct drm_plane *plane;
   472		unsigned int num_planes = 0;
   473		unsigned int num_alpha_planes = 0;
   474		unsigned int num_frontend_planes = 0;
   475		unsigned int num_alpha_planes_max = 1;
   476		unsigned int num_yuv_planes = 0;
   477		unsigned int current_pipe = 0;
   478		unsigned int i;
   479	
   480		DRM_DEBUG_DRIVER("Starting checking our planes\n");
   481	
   482		if (!crtc_state->planes_changed)
   483			return 0;
   484	
   485		drm_for_each_plane_mask(plane, drm, crtc_state->plane_mask) {
   486			struct drm_plane_state *plane_state =
   487				drm_atomic_get_plane_state(state, plane);
   488			struct sun4i_layer_state *layer_state =
   489				state_to_sun4i_layer_state(plane_state);
   490			struct drm_framebuffer *fb = plane_state->fb;
   491			struct drm_format_name_buf format_name;
   492	
   493			if (sun4i_backend_plane_uses_frontend(plane_state)) {
   494				DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
   495						 plane->index);
   496	
   497				layer_state->uses_frontend = true;
   498				num_frontend_planes++;
   499			} else {
   500				layer_state->uses_frontend = false;
   501			}
   502	
   503			DRM_DEBUG_DRIVER("Plane FB format is %s\n",
   504					 drm_get_format_name(fb->format->format,
   505							     &format_name));
   506			if (fb->format->has_alpha || (plane_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
   507				num_alpha_planes++;
   508	
   509			if (sun4i_backend_format_is_yuv(fb->format->format)) {
   510				DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
   511				num_yuv_planes++;
   512			}
   513	
   514			DRM_DEBUG_DRIVER("Plane zpos is %d\n",
   515					 plane_state->normalized_zpos);
   516	
   517			/* Sort our planes by Zpos */
   518			plane_states[plane_state->normalized_zpos] = plane_state;
   519	
   520			num_planes++;
   521		}
   522	
   523		/* All our planes were disabled, bail out */
   524		if (!num_planes)
   525			return 0;
   526	
   527		/*
   528		 * The hardware is a bit unusual here.
   529		 *
   530		 * Even though it supports 4 layers, it does the composition
   531		 * in two separate steps.
   532		 *
   533		 * The first one is assigning a layer to one of its two
   534		 * pipes. If more that 1 layer is assigned to the same pipe,
   535		 * and if pixels overlaps, the pipe will take the pixel from
   536		 * the layer with the highest priority.
   537		 *
   538		 * The second step is the actual alpha blending, that takes
   539		 * the two pipes as input, and uses the potential alpha
   540		 * component to do the transparency between the two.
   541		 *
   542		 * This two-step scenario makes us unable to guarantee a
   543		 * robust alpha blending between the 4 layers in all
   544		 * situations, since this means that we need to have one layer
   545		 * with alpha at the lowest position of our two pipes.
   546		 *
   547		 * However, we cannot even do that on every platform, since the
   548		 * hardware has a bug where the lowest plane of the lowest pipe
   549		 * (pipe 0, priority 0), if it has any alpha, will discard the
   550		 * pixel data entirely and just display the pixels in the
   551		 * background color (black by default).
   552		 *
   553		 * This means that on the affected platforms, we effectively
   554		 * have only three valid configurations with alpha, all of them
   555		 * with the alpha being on pipe1 with the lowest position, which
   556		 * can be 1, 2 or 3 depending on the number of planes and their zpos.
   557		 */
   558	
   559		/* For platforms that are not affected by the issue described above. */
 > 560		if (backend->quirks->supports_lowest_plane_alpha)
   561			num_alpha_planes_max++;
   562	
   563		if (num_alpha_planes > num_alpha_planes_max) {
   564			DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
   565			return -EINVAL;
   566		}
   567	
   568		/* We can't have an alpha plane at the lowest position */
   569		if (!backend->quirks->supports_lowest_plane_alpha &&
   570		    (plane_states[0]->fb->format->has_alpha ||
   571		    (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)))
   572			return -EINVAL;
   573	
   574		for (i = 1; i < num_planes; i++) {
   575			struct drm_plane_state *p_state = plane_states[i];
   576			struct drm_framebuffer *fb = p_state->fb;
   577			struct sun4i_layer_state *s_state = state_to_sun4i_layer_state(p_state);
   578	
   579			/*
   580			 * The only alpha position is the lowest plane of the
   581			 * second pipe.
   582			 */
   583			if (fb->format->has_alpha || (p_state->alpha != DRM_BLEND_ALPHA_OPAQUE))
   584				current_pipe++;
   585	
   586			s_state->pipe = current_pipe;
   587		}
   588	
   589		/* We can only have a single YUV plane at a time */
   590		if (num_yuv_planes > SUN4I_BACKEND_NUM_YUV_PLANES) {
   591			DRM_DEBUG_DRIVER("Too many planes with YUV, rejecting...\n");
   592			return -EINVAL;
   593		}
   594	
   595		if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
   596			DRM_DEBUG_DRIVER("Too many planes going through the frontend, rejecting\n");
   597			return -EINVAL;
   598		}
   599	
   600		DRM_DEBUG_DRIVER("State valid with %u planes, %u alpha, %u video, %u YUV\n",
   601				 num_planes, num_alpha_planes, num_frontend_planes,
   602				 num_yuv_planes);
   603	
   604		return 0;
   605	}
   606	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66067 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel