2018-01-22 10:43:18

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 00/19] drm/sun4i: Support more planes, zpos and plane-wide alpha

Hi,

This serie aims at enhancing the support for our planes in the current drm
driver on the first generation of Allwinner's display engine.

This also introduces a few generic stuff, as well as some conversion for
some other drivers.

This series basically implements three things that look orthogonal, but due
to the way the hardware works are kind of related.

The main feature is that instead of implementing 2 planes per backend, we
are now able to use the planes that are available in hardware. This was
unsupported before because of the way the composition works in the
hardware.

Indeed, the planes are first grouped into 2 pipes that are doing a basic
composition, in case of overlapping planes, it just takes whatever plane
has the highest priority (=> zpos). Then, the alpha blending is done
between the two pipes. This was simplified so far by only using two planes,
one for each pipe, which was allowing us to have an illusion of proper
alpha blending. This is further complicated by the bug/feature that the
lowest plane must not have any alpha at all, otherwise the pixel will turn
black, no matter what the value of alpha is. This basically means that we
can have a plane with alpha only in the second pipe.

However, as we have more and more blocks being worked on, 2 planes are
getting really limited and we need to support all 4 of them.

This is mostly possible by extending our atomic_check and to make sure that
we enforce those constraints, and assign the pipes automatically. This is
done by looking at the number of planes using an alpha component, and we
then end up in various scenarios:
- 0 plane with alpha
=> we don't care for the pipes at all. All the planes are assigned to
the first pipe
- 1 plane with alpha
=> we assign all the planes without alpha below the plane with alpha to
the first pipe, and then all the remaining planes to the second
pipe. The plane with alpha will be the lowest plane on that pipe,
which means that whatever plane is above it will have precedence,
but the alpha component will remain and will be used on pixels that
are not overlapping
- 2-4 planes with alpha
=> we can't operate that way, we just reject the configuration.

In addition to the formats that embed an alpha component, we also add
support for plane-wide alpha property, and in order to tweak the
configuration the way we want to, we also add support for configurable
zpos.

Let me know what you think,
Maxime

Changes from v1:
- Document the behaviour on concurrent usage of the alpha property and an
alpha component in the format
- Allowed for higher alpha values
- Moved the alpha value from a helper to the struct drm_format_info
- Collected tags

Maxime Ripard (19):
drm/fourcc: Add a alpha field to drm_format_info
drm/atmel-hlcdc: Use the alpha format field in drm_format_info
drm/atmel-exynos: Use the alpha format field in drm_format_info
drm/rockchip: Use the alpha format field in drm_format_info
drm/vc4: Use the alpha format field in drm_format_info
drm/blend: Add a generic alpha property
drm/atmel-hclcdc: Convert to the new generic alpha property
drm/rcar-du: Convert to the new generic alpha property
drm/sun4i: backend: Fix structure indentation
drm/sun4i: backend: Fix define typo
drm/sun4i: framebuffer: Add a custom atomic_check
drm/sun4i: backend: Move the coord function in the shared part
drm/sun4i: backend: Set a default zpos in our reset hook
drm/sun4i: backend: Add support for zpos
drm/sun4i: backend: Check for the number of alpha planes
drm/sun4i: backend: Assign the pipes automatically
drm/sun4i: backend: Make zpos configurable
drm/sun4i: Add support for plane alpha
drm/sun4i: backend: Remove ARGB spoofing

Documentation/gpu/kms-properties.csv | 2 +-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 13 +--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 113 ++-------------
drivers/gpu/drm/drm_atomic.c | 4 +-
drivers/gpu/drm/drm_atomic_helper.c | 4 +-
drivers/gpu/drm/drm_blend.c | 32 ++++-
drivers/gpu/drm/drm_fourcc.c | 50 +++----
drivers/gpu/drm/exynos/exynos_mixer.c | 14 +--
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 5 +-
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 15 +--
drivers/gpu/drm/rcar-du/rcar_du_plane.h | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 42 +------
drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 +--
drivers/gpu/drm/sun4i/sun4i_backend.c | 124 +++++++++++++++--
drivers/gpu/drm/sun4i/sun4i_backend.h | 11 +-
drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 18 +-
drivers/gpu/drm/sun4i/sun4i_layer.c | 84 ++----------
drivers/gpu/drm/sun4i/sun4i_layer.h | 1 +-
drivers/gpu/drm/vc4/vc4_plane.c | 19 +--
include/drm/drm_blend.h | 1 +-
include/drm/drm_fourcc.h | 2 +-
include/drm/drm_plane.h | 6 +-
24 files changed, 275 insertions(+), 303 deletions(-)

base-commit: 53b519deaf2e507b121b64abcb4ba0da075bd6a7
--
git-series 0.9.1


2018-01-22 10:37:02

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 11/19] drm/sun4i: framebuffer: Add a custom atomic_check

In order to support normalized zpos, we need to call
drm_atomic_normalize_zpos in our driver's drm_mode_config_funcs'
atomic_check.

Let's duplicate the definition of drm_atomic_helper_check for now.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
index a01a5b7d46e6..e68004844abe 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
@@ -26,9 +26,21 @@ static void sun4i_de_output_poll_changed(struct drm_device *drm)
drm_fbdev_cma_hotplug_event(drv->fbdev);
}

+static int sun4i_de_atomic_check(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ int ret;
+
+ ret = drm_atomic_helper_check_modeset(dev, state);
+ if (ret)
+ return ret;
+
+ return drm_atomic_helper_check_planes(dev, state);
+}
+
static const struct drm_mode_config_funcs sun4i_de_mode_config_funcs = {
.output_poll_changed = sun4i_de_output_poll_changed,
- .atomic_check = drm_atomic_helper_check,
+ .atomic_check = sun4i_de_atomic_check,
.atomic_commit = drm_atomic_helper_commit,
.fb_create = drm_gem_fb_create,
};
--
git-series 0.9.1

2018-01-22 10:37:50

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 18/19] drm/sun4i: Add support for plane alpha

Our backend supports a per-plane alpha property. Support it through our new
helper.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_backend.c | 16 +++++++++++++---
drivers/gpu/drm/sun4i/sun4i_backend.h | 3 +++
drivers/gpu/drm/sun4i/sun4i_layer.c | 2 ++
3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 38c4b44f6ff5..f5beeec06fd5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -191,6 +191,15 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
DRM_DEBUG_DRIVER("Switching display backend interlaced mode %s\n",
interlaced ? "on" : "off");

+ val = SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA(state->alpha);
+ if (state->alpha != 255)
+ val |= SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN;
+ regmap_update_bits(backend->engine.regs,
+ SUN4I_BACKEND_ATTCTL_REG0(layer),
+ SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_MASK |
+ SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
+ val);
+
ret = sun4i_backend_drm_format_to_layer(plane, fb->format->format,
&val);
if (ret) {
@@ -366,7 +375,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
DRM_DEBUG_DRIVER("Plane FB format is %s\n",
drm_get_format_name(fb->format->format,
&format_name));
- if (fb->format->has_alpha)
+ if (fb->format->has_alpha || (plane_state->alpha != 255))
num_alpha_planes++;

DRM_DEBUG_DRIVER("Plane zpos is %d\n",
@@ -419,7 +428,8 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
}

/* We can't have an alpha plane at the lowest position */
- if (plane_states[0]->fb->format->has_alpha)
+ if (plane_states[0]->fb->format->has_alpha ||
+ (plane_states[0]->alpha != 255))
return -EINVAL;

for (i = 1; i < num_planes; i++) {
@@ -431,7 +441,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
* The only alpha position is the lowest plane of the
* second pipe.
*/
- if (fb->format->has_alpha)
+ if (fb->format->has_alpha || (p_state->alpha != 255))
current_pipe++;

s_state->pipe = current_pipe;
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 52e77591186a..03294d5dd1a2 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -68,11 +68,14 @@
#define SUN4I_BACKEND_CKMIN_REG 0x884
#define SUN4I_BACKEND_CKCFG_REG 0x888
#define SUN4I_BACKEND_ATTCTL_REG0(l) (0x890 + (0x4 * (l)))
+#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_MASK GENMASK(31, 24)
+#define SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA(x) ((x) << 24)
#define SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK BIT(15)
#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_REG0_LAY_GLBALPHA_EN BIT(0)

#define SUN4I_BACKEND_ATTCTL_REG1(l) (0x8a0 + (0x4 * (l)))
#define SUN4I_BACKEND_ATTCTL_REG1_LAY_HSCAFCT GENMASK(15, 14)
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 9e538f761dcb..d5598de92f85 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -37,6 +37,7 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
if (state) {
plane->state = &state->state;
plane->state->plane = plane;
+ plane->state->alpha = 255;
plane->state->zpos = layer->id;
}
}
@@ -163,6 +164,7 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
&sun4i_backend_layer_helper_funcs);
layer->backend = backend;

+ drm_plane_create_alpha_property(&layer->plane, 255);
drm_plane_create_zpos_property(&layer->plane, 0, 0,
SUN4I_BACKEND_NUM_LAYERS - 1);

--
git-series 0.9.1

2018-01-22 10:38:18

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 14/19] drm/sun4i: backend: Add support for zpos

Our various planes have a configurable zpos, that combined with the pipes
allow to configure the composition.

Since the interaction between the pipes, zpos and alphas framebuffers are
not trivials, let's just enable the zpos as an immutable property for now,
and use that zpos in our atomic_update part.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_backend.c | 15 +++++++++++++++
drivers/gpu/drm/sun4i/sun4i_backend.h | 2 ++
drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 4 ++++
drivers/gpu/drm/sun4i/sun4i_layer.c | 3 +++
4 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index a18c86a15748..c4986054909b 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -272,6 +272,21 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
return 0;
}

+int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend, int layer,
+ struct drm_plane *plane)
+{
+ struct drm_plane_state *state = plane->state;
+ unsigned int priority = state->normalized_zpos;
+
+ DRM_DEBUG_DRIVER("Setting layer %d priority to %d\n", layer, priority);
+
+ regmap_update_bits(backend->engine.regs, SUN4I_BACKEND_ATTCTL_REG0(layer),
+ SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK,
+ SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(priority));
+
+ return 0;
+}
+
static bool sun4i_backend_plane_uses_scaler(struct drm_plane_state *state)
{
u16 src_h = state->src_h >> 16;
diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 1ca8b7db6807..04a4f11b87a8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -182,5 +182,7 @@ 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);
+int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend,
+ int layer, struct drm_plane *plane);

#endif /* _SUN4I_BACKEND_H_ */
diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
index e68004844abe..5b3986437a50 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
@@ -35,6 +35,10 @@ static int sun4i_de_atomic_check(struct drm_device *dev,
if (ret)
return ret;

+ ret = drm_atomic_normalize_zpos(dev, state);
+ if (ret)
+ return ret;
+
return drm_atomic_helper_check_planes(dev, state);
}

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 03549646528a..fbf25d59cf88 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -115,6 +115,7 @@ static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
}

sun4i_backend_update_layer_coord(backend, layer->id, plane);
+ sun4i_backend_update_layer_zpos(backend, layer->id, plane);
sun4i_backend_layer_enable(backend, layer->id, true);
}

@@ -237,6 +238,8 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
return ERR_CAST(layer);
};

+ drm_plane_create_zpos_immutable_property(&layer->plane, i);
+
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),
--
git-series 0.9.1

2018-01-22 10:38:44

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 17/19] drm/sun4i: backend: Make zpos configurable

Now that we have everything in place, we can make zpos configurable now.
Change the zpos property from an immutable one to a regular.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_layer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index ec7b906dbb84..9e538f761dcb 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -163,6 +163,9 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
&sun4i_backend_layer_helper_funcs);
layer->backend = backend;

+ drm_plane_create_zpos_property(&layer->plane, 0, 0,
+ SUN4I_BACKEND_NUM_LAYERS - 1);
+
return layer;
}

@@ -189,8 +192,6 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
return ERR_CAST(layer);
};

- drm_plane_create_zpos_immutable_property(&layer->plane, i);
-
layer->id = i;
planes[i] = &layer->plane;
};
--
git-series 0.9.1

2018-01-22 10:38:47

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 12/19] drm/sun4i: backend: Move the coord function in the shared part

The function supposed to update a plane's coordinates is called in both
branches of our function. Let's move it out the if statement.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_layer.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index f03da16eb92a..c448cb6b9fa9 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -106,14 +106,13 @@ static void sun4i_backend_layer_atomic_update(struct drm_plane *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_layer_enable(backend, layer->id, true);
}

--
git-series 0.9.1

2018-01-22 10:38:50

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 15/19] drm/sun4i: backend: Check for the number of alpha planes

Due to the way the composition is done in hardware, we can only have a
single alpha-enabled plane active at a time, placed in the second (highest
priority) pipe.

Make sure of that in our atomic_check to not end up in an impossible
scenario.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_backend.c | 50 ++++++++++++++++++++++++++++-
drivers/gpu/drm/sun4i/sun4i_backend.h | 2 +-
drivers/gpu/drm/sun4i/sun4i_layer.c | 23 +-------------
3 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index c4986054909b..eb1749d2c0d5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -329,6 +329,8 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *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;

DRM_DEBUG_DRIVER("Starting checking our planes\n");
@@ -341,6 +343,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
drm_atomic_get_plane_state(state, plane);
struct sun4i_layer_state *layer_state =
state_to_sun4i_layer_state(plane_state);
+ struct drm_framebuffer *fb = plane_state->fb;

if (sun4i_backend_plane_uses_frontend(plane_state)) {
DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
@@ -351,6 +354,50 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
} else {
layer_state->uses_frontend = false;
}
+
+ DRM_DEBUG_DRIVER("Plane FB format is %s\n",
+ drm_get_format_name(fb->format->format,
+ &format_name));
+ if (fb->format->has_alpha)
+ num_alpha_planes++;
+
+ num_planes++;
+ }
+
+ /*
+ * 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, 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).
+ *
+ * Since 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.
+ */
+ if (num_alpha_planes > SUN4I_BACKEND_NUM_ALPHA_LAYERS) {
+ DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
+ return -EINVAL;
}

if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
@@ -358,6 +405,9 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
return -EINVAL;
}

+ DRM_DEBUG_DRIVER("State valid with %u planes, %u alpha, %u video\n",
+ num_planes, num_alpha_planes, num_frontend_planes);
+
return 0;
}

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index 04a4f11b87a8..52e77591186a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -146,6 +146,8 @@
#define SUN4I_BACKEND_HWCCOLORTAB_OFF 0x4c00
#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

struct sun4i_backend {
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index fbf25d59cf88..900e716443b8 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -201,32 +201,11 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
int i;

- planes = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes) + 1,
+ planes = devm_kcalloc(drm->dev, SUN4I_BACKEND_NUM_LAYERS,
sizeof(*planes), GFP_KERNEL);
if (!planes)
return ERR_PTR(-ENOMEM);

- /*
- * 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;
--
git-series 0.9.1

2018-01-22 10:39:16

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 16/19] drm/sun4i: backend: Assign the pipes automatically

Since we now have a way to enforce the zpos, check for the number of alpha
planes, the only missing part is to assign our pipe automatically instead
of hardcoding it.

The algorithm is quite simple, but requires two iterations over the list of
planes.

In the first one (which is the same one that we've had to check for alpha,
the frontend usage, and so on), we order the planes by their zpos.

We can then do a second iteration over that array by ascending zpos
starting with the pipe 0. When and if we encounter our alpha plane, we put
it and all the other subsequent planes in the second pipe.

And since we have runtime checks and pipe assignments now, we can just
remove the static declaration of the planes we used to have.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_backend.c | 41 +++++++++++++++++++++--
drivers/gpu/drm/sun4i/sun4i_layer.c | 50 ++++------------------------
drivers/gpu/drm/sun4i/sun4i_layer.h | 1 +-
3 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index eb1749d2c0d5..38c4b44f6ff5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -276,12 +276,16 @@ int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend, int layer,
struct drm_plane *plane)
{
struct drm_plane_state *state = plane->state;
+ struct sun4i_layer_state *p_state = state_to_sun4i_layer_state(state);
unsigned int priority = state->normalized_zpos;
+ unsigned int pipe = p_state->pipe;

- DRM_DEBUG_DRIVER("Setting layer %d priority to %d\n", layer, priority);
-
+ DRM_DEBUG_DRIVER("Setting layer %d priority to %d and pipe %d\n",
+ layer, priority, pipe);
regmap_update_bits(backend->engine.regs, SUN4I_BACKEND_ATTCTL_REG0(layer),
+ SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL_MASK |
SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK,
+ SUN4I_BACKEND_ATTCTL_REG0_LAY_PIPESEL(p_state->pipe) |
SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(priority));

return 0;
@@ -326,12 +330,15 @@ static void sun4i_backend_atomic_begin(struct sunxi_engine *engine,
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 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 current_pipe = 0;
+ unsigned int i;

DRM_DEBUG_DRIVER("Starting checking our planes\n");

@@ -344,6 +351,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
struct sun4i_layer_state *layer_state =
state_to_sun4i_layer_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_format_name_buf format_name;

if (sun4i_backend_plane_uses_frontend(plane_state)) {
DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
@@ -361,9 +369,19 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
if (fb->format->has_alpha)
num_alpha_planes++;

+ DRM_DEBUG_DRIVER("Plane zpos is %d\n",
+ plane_state->normalized_zpos);
+
+ /* Sort our planes by Zpos */
+ plane_states[plane_state->normalized_zpos] = plane_state;
+
num_planes++;
}

+ /* All our planes were disabled, bail out */
+ if (!num_planes)
+ return 0;
+
/*
* The hardware is a bit unusual here.
*
@@ -400,6 +418,25 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
return -EINVAL;
}

+ /* We can't have an alpha plane at the lowest position */
+ if (plane_states[0]->fb->format->has_alpha)
+ return -EINVAL;
+
+ for (i = 1; i < num_planes; i++) {
+ struct drm_plane_state *p_state = plane_states[i];
+ struct drm_framebuffer *fb = p_state->fb;
+ struct sun4i_layer_state *s_state = state_to_sun4i_layer_state(p_state);
+
+ /*
+ * The only alpha position is the lowest plane of the
+ * second pipe.
+ */
+ if (fb->format->has_alpha)
+ current_pipe++;
+
+ s_state->pipe = current_pipe;
+ }
+
if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
DRM_DEBUG_DRIVER("Too many planes going through the frontend, rejecting\n");
return -EINVAL;
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 900e716443b8..ec7b906dbb84 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -19,13 +19,6 @@
#include "sun4i_layer.h"
#include "sunxi_engine.h"

-struct sun4i_plane_desc {
- enum drm_plane_type type;
- u8 pipe;
- const uint32_t *formats;
- uint32_t nformats;
-};
-
static void sun4i_backend_layer_reset(struct drm_plane *plane)
{
struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
@@ -133,14 +126,7 @@ static const struct drm_plane_funcs sun4i_backend_layer_funcs = {
.update_plane = drm_atomic_helper_update_plane,
};

-static const uint32_t sun4i_backend_layer_formats_primary[] = {
- DRM_FORMAT_ARGB8888,
- DRM_FORMAT_RGB888,
- DRM_FORMAT_RGB565,
- DRM_FORMAT_XRGB8888,
-};
-
-static const uint32_t sun4i_backend_layer_formats_overlay[] = {
+static const uint32_t sun4i_backend_layer_formats[] = {
DRM_FORMAT_ARGB8888,
DRM_FORMAT_ARGB4444,
DRM_FORMAT_ARGB1555,
@@ -151,24 +137,9 @@ static const uint32_t sun4i_backend_layer_formats_overlay[] = {
DRM_FORMAT_XRGB8888,
};

-static const struct sun4i_plane_desc sun4i_backend_planes[] = {
- {
- .type = DRM_PLANE_TYPE_PRIMARY,
- .pipe = 0,
- .formats = sun4i_backend_layer_formats_primary,
- .nformats = ARRAY_SIZE(sun4i_backend_layer_formats_primary),
- },
- {
- .type = DRM_PLANE_TYPE_OVERLAY,
- .pipe = 1,
- .formats = sun4i_backend_layer_formats_overlay,
- .nformats = ARRAY_SIZE(sun4i_backend_layer_formats_overlay),
- },
-};
-
static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
struct sun4i_backend *backend,
- const struct sun4i_plane_desc *plane)
+ enum drm_plane_type type)
{
struct sun4i_layer *layer;
int ret;
@@ -180,8 +151,9 @@ static struct sun4i_layer *sun4i_layer_init_one(struct drm_device *drm,
/* possible crtcs are set later */
ret = drm_universal_plane_init(drm, &layer->plane, 0,
&sun4i_backend_layer_funcs,
- plane->formats, plane->nformats,
- NULL, plane->type, NULL);
+ sun4i_backend_layer_formats,
+ ARRAY_SIZE(sun4i_backend_layer_formats),
+ NULL, type, NULL);
if (ret) {
dev_err(drm->dev, "Couldn't initialize layer\n");
return ERR_PTR(ret);
@@ -206,11 +178,11 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
if (!planes)
return ERR_PTR(-ENOMEM);

- for (i = 0; i < ARRAY_SIZE(sun4i_backend_planes); i++) {
- const struct sun4i_plane_desc *plane = &sun4i_backend_planes[i];
+ for (i = 0; i < SUN4I_BACKEND_NUM_LAYERS; i++) {
+ enum drm_plane_type type = i ? DRM_PLANE_TYPE_OVERLAY : DRM_PLANE_TYPE_PRIMARY;
struct sun4i_layer *layer;

- layer = sun4i_layer_init_one(drm, backend, plane);
+ layer = sun4i_layer_init_one(drm, backend, type);
if (IS_ERR(layer)) {
dev_err(drm->dev, "Couldn't initialize %s plane\n",
i ? "overlay" : "primary");
@@ -219,12 +191,6 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,

drm_plane_create_zpos_immutable_property(&layer->plane, i);

- 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;
};
diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h b/drivers/gpu/drm/sun4i/sun4i_layer.h
index 75b4868ba87c..36b20265bd31 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;
+ unsigned int pipe;
bool uses_frontend;
};

--
git-series 0.9.1

2018-01-22 10:40:12

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 09/19] drm/sun4i: backend: Fix structure indentation

The sun4i_plane_desc structure was somehow indented to two tabulations
instead of one as we shoud do. Fix that.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_layer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index 4652b25be0d2..f03da16eb92a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -20,10 +20,10 @@
#include "sunxi_engine.h"

struct sun4i_plane_desc {
- enum drm_plane_type type;
- u8 pipe;
- const uint32_t *formats;
- uint32_t nformats;
+ enum drm_plane_type type;
+ u8 pipe;
+ const uint32_t *formats;
+ uint32_t nformats;
};

static void sun4i_backend_layer_reset(struct drm_plane *plane)
--
git-series 0.9.1

2018-01-22 10:40:15

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 07/19] drm/atmel-hclcdc: Convert to the new generic alpha property

Now that we have support for per-plane alpha in the core, let's use it.

Acked-by: Boris Brezillon <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 13 +---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 89 ++----------------
2 files changed, 14 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 6833ee253cfa..704cac6399eb 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -298,7 +298,6 @@ struct atmel_hlcdc_layer {
struct atmel_hlcdc_plane {
struct drm_plane base;
struct atmel_hlcdc_layer layer;
- struct atmel_hlcdc_plane_properties *properties;
};

static inline struct atmel_hlcdc_plane *
@@ -345,18 +344,6 @@ struct atmel_hlcdc_dc_desc {
};

/**
- * Atmel HLCDC Plane properties.
- *
- * This structure stores plane property definitions.
- *
- * @alpha: alpha blending (or transparency) property
- * @rotation: rotation property
- */
-struct atmel_hlcdc_plane_properties {
- struct drm_property *alpha;
-};
-
-/**
* Atmel HLCDC Display Controller.
*
* @desc: HLCDC Display Controller description
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index e18800ed7cd1..d1b89a6f6280 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -31,7 +31,6 @@
* @src_y: y buffer position
* @src_w: buffer width
* @src_h: buffer height
- * @alpha: alpha blending of the plane
* @disc_x: x discard position
* @disc_y: y discard position
* @disc_w: discard width
@@ -54,8 +53,6 @@ struct atmel_hlcdc_plane_state {
uint32_t src_w;
uint32_t src_h;

- u8 alpha;
-
int disc_x;
int disc_y;
int disc_w;
@@ -385,7 +382,7 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
cfg |= ATMEL_HLCDC_LAYER_LAEN;
else
cfg |= ATMEL_HLCDC_LAYER_GAEN |
- ATMEL_HLCDC_LAYER_GA(state->alpha);
+ ATMEL_HLCDC_LAYER_GA(state->base.alpha);
}

if (state->disc_h && state->disc_w)
@@ -553,7 +550,7 @@ atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state)

if (!ovl_s->fb ||
ovl_s->fb->format->has_alpha ||
- ovl_state->alpha != 255)
+ ovl_s->alpha != 255)
continue;

/* TODO: implement a smarter hidden area detection */
@@ -829,51 +826,18 @@ static void atmel_hlcdc_plane_destroy(struct drm_plane *p)
drm_plane_cleanup(p);
}

-static int atmel_hlcdc_plane_atomic_set_property(struct drm_plane *p,
- struct drm_plane_state *s,
- struct drm_property *property,
- uint64_t val)
-{
- struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
- struct atmel_hlcdc_plane_properties *props = plane->properties;
- struct atmel_hlcdc_plane_state *state =
- drm_plane_state_to_atmel_hlcdc_plane_state(s);
-
- if (property == props->alpha)
- state->alpha = val;
- else
- return -EINVAL;
-
- return 0;
-}
-
-static int atmel_hlcdc_plane_atomic_get_property(struct drm_plane *p,
- const struct drm_plane_state *s,
- struct drm_property *property,
- uint64_t *val)
-{
- struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
- struct atmel_hlcdc_plane_properties *props = plane->properties;
- const struct atmel_hlcdc_plane_state *state =
- container_of(s, const struct atmel_hlcdc_plane_state, base);
-
- if (property == props->alpha)
- *val = state->alpha;
- else
- return -EINVAL;
-
- return 0;
-}
-
-static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane,
- struct atmel_hlcdc_plane_properties *props)
+static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
{
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;

if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
- desc->type == ATMEL_HLCDC_CURSOR_LAYER)
- drm_object_attach_property(&plane->base.base,
- props->alpha, 255);
+ desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
+ int ret;
+
+ ret = drm_plane_create_alpha_property(&plane->base, 255);
+ if (ret)
+ return ret;
+ }

if (desc->layout.xstride && desc->layout.pstride) {
int ret;
@@ -988,8 +952,8 @@ static void atmel_hlcdc_plane_reset(struct drm_plane *p)
return;
}

- state->alpha = 255;
p->state = &state->base;
+ p->state->alpha = 255;
p->state->plane = p;
}
}
@@ -1042,13 +1006,10 @@ static const struct drm_plane_funcs layer_plane_funcs = {
.reset = atmel_hlcdc_plane_reset,
.atomic_duplicate_state = atmel_hlcdc_plane_atomic_duplicate_state,
.atomic_destroy_state = atmel_hlcdc_plane_atomic_destroy_state,
- .atomic_set_property = atmel_hlcdc_plane_atomic_set_property,
- .atomic_get_property = atmel_hlcdc_plane_atomic_get_property,
};

static int atmel_hlcdc_plane_create(struct drm_device *dev,
- const struct atmel_hlcdc_layer_desc *desc,
- struct atmel_hlcdc_plane_properties *props)
+ const struct atmel_hlcdc_layer_desc *desc)
{
struct atmel_hlcdc_dc *dc = dev->dev_private;
struct atmel_hlcdc_plane *plane;
@@ -1060,7 +1021,6 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
return -ENOMEM;

atmel_hlcdc_layer_init(&plane->layer, desc, dc->hlcdc->regmap);
- plane->properties = props;

if (desc->type == ATMEL_HLCDC_BASE_LAYER)
type = DRM_PLANE_TYPE_PRIMARY;
@@ -1081,7 +1041,7 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
&atmel_hlcdc_layer_plane_helper_funcs);

/* Set default property values*/
- ret = atmel_hlcdc_plane_init_properties(plane, props);
+ ret = atmel_hlcdc_plane_init_properties(plane);
if (ret)
return ret;

@@ -1090,34 +1050,13 @@ static int atmel_hlcdc_plane_create(struct drm_device *dev,
return 0;
}

-static struct atmel_hlcdc_plane_properties *
-atmel_hlcdc_plane_create_properties(struct drm_device *dev)
-{
- struct atmel_hlcdc_plane_properties *props;
-
- props = devm_kzalloc(dev->dev, sizeof(*props), GFP_KERNEL);
- if (!props)
- return ERR_PTR(-ENOMEM);
-
- props->alpha = drm_property_create_range(dev, 0, "alpha", 0, 255);
- if (!props->alpha)
- return ERR_PTR(-ENOMEM);
-
- return props;
-}
-
int atmel_hlcdc_create_planes(struct drm_device *dev)
{
struct atmel_hlcdc_dc *dc = dev->dev_private;
- struct atmel_hlcdc_plane_properties *props;
const struct atmel_hlcdc_layer_desc *descs = dc->desc->layers;
int nlayers = dc->desc->nlayers;
int i, ret;

- props = atmel_hlcdc_plane_create_properties(dev);
- if (IS_ERR(props))
- return PTR_ERR(props);
-
dc->dscrpool = dmam_pool_create("atmel-hlcdc-dscr", dev->dev,
sizeof(struct atmel_hlcdc_dma_channel_dscr),
sizeof(u64), 0);
@@ -1130,7 +1069,7 @@ int atmel_hlcdc_create_planes(struct drm_device *dev)
descs[i].type != ATMEL_HLCDC_CURSOR_LAYER)
continue;

- ret = atmel_hlcdc_plane_create(dev, &descs[i], props);
+ ret = atmel_hlcdc_plane_create(dev, &descs[i]);
if (ret)
return ret;
}
--
git-series 0.9.1

2018-01-22 10:40:20

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 08/19] drm/rcar-du: Convert to the new generic alpha property

Now that we have support for per-plane alpha in the core, let's use it.

Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +-
drivers/gpu/drm/rcar-du/rcar_du_kms.c | 5 +---
drivers/gpu/drm/rcar-du/rcar_du_plane.c | 15 +++------
drivers/gpu/drm/rcar-du/rcar_du_plane.h | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 42 ++------------------------
drivers/gpu/drm/rcar-du/rcar_du_vsp.h | 2 +-
6 files changed, 9 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index f8cd79488ece..aff04adaae53 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -89,7 +89,6 @@ struct rcar_du_device {
struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];

struct {
- struct drm_property *alpha;
struct drm_property *colorkey;
} props;

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 566d1a948c8f..e1b5a7b460cc 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -417,11 +417,6 @@ static int rcar_du_encoders_init(struct rcar_du_device *rcdu)

static int rcar_du_properties_init(struct rcar_du_device *rcdu)
{
- rcdu->props.alpha =
- drm_property_create_range(rcdu->ddev, 0, "alpha", 0, 255);
- if (rcdu->props.alpha == NULL)
- return -ENOMEM;
-
/*
* The color key is expressed as an RGB888 triplet stored in a 32-bit
* integer in XRGB8888 format. Bit 24 is used as a flag to disable (0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 61833cc1c699..5b34e8092c8b 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -423,7 +423,7 @@ static void rcar_du_plane_setup_mode(struct rcar_du_group *rgrp,
rcar_du_plane_write(rgrp, index, PnALPHAR, PnALPHAR_ABIT_0);
else
rcar_du_plane_write(rgrp, index, PnALPHAR,
- PnALPHAR_ABIT_X | state->alpha);
+ PnALPHAR_ABIT_X | state->state.alpha);

pnmr = PnMR_BM_MD | state->format->pnmr;

@@ -667,11 +667,11 @@ static void rcar_du_plane_reset(struct drm_plane *plane)

state->hwindex = -1;
state->source = RCAR_DU_PLANE_MEMORY;
- state->alpha = 255;
state->colorkey = RCAR_DU_COLORKEY_NONE;
state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;

plane->state = &state->state;
+ plane->state->alpha = 255;
plane->state->plane = plane;
}

@@ -683,9 +683,7 @@ static int rcar_du_plane_atomic_set_property(struct drm_plane *plane,
struct rcar_du_plane_state *rstate = to_rcar_plane_state(state);
struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;

- if (property == rcdu->props.alpha)
- rstate->alpha = val;
- else if (property == rcdu->props.colorkey)
+ if (property == rcdu->props.colorkey)
rstate->colorkey = val;
else
return -EINVAL;
@@ -701,9 +699,7 @@ static int rcar_du_plane_atomic_get_property(struct drm_plane *plane,
container_of(state, const struct rcar_du_plane_state, state);
struct rcar_du_device *rcdu = to_rcar_plane(plane)->group->dev;

- if (property == rcdu->props.alpha)
- *val = rstate->alpha;
- else if (property == rcdu->props.colorkey)
+ if (property == rcdu->props.colorkey)
*val = rstate->colorkey;
else
return -EINVAL;
@@ -772,10 +768,9 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp)
continue;

drm_object_attach_property(&plane->plane.base,
- rcdu->props.alpha, 255);
- drm_object_attach_property(&plane->plane.base,
rcdu->props.colorkey,
RCAR_DU_COLORKEY_NONE);
+ drm_plane_create_alpha_property(&plane->plane, 255);
drm_plane_create_zpos_property(&plane->plane, 1, 1, 7);
}

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index f62e09f195de..2dc793ebd1a2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -50,7 +50,6 @@ static inline struct rcar_du_plane *to_rcar_plane(struct drm_plane *plane)
* @state: base DRM plane state
* @format: information about the pixel format used by the plane
* @hwindex: 0-based hardware plane index, -1 means unused
- * @alpha: value of the plane alpha property
* @colorkey: value of the plane colorkey property
*/
struct rcar_du_plane_state {
@@ -60,7 +59,6 @@ struct rcar_du_plane_state {
int hwindex;
enum rcar_du_plane_source source;

- unsigned int alpha;
unsigned int colorkey;
};

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 2c96147bc444..ee85f6fdffad 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -54,6 +54,7 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
};
struct rcar_du_plane_state state = {
.state = {
+ .alpha = 255,
.crtc = &crtc->crtc,
.crtc_x = 0,
.crtc_y = 0,
@@ -67,7 +68,6 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
},
.format = rcar_du_format_info(DRM_FORMAT_ARGB8888),
.source = RCAR_DU_PLANE_VSPD1,
- .alpha = 255,
.colorkey = 0,
};

@@ -173,7 +173,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
struct vsp1_du_atomic_config cfg = {
.pixelformat = 0,
.pitch = fb->pitches[0],
- .alpha = state->alpha,
+ .alpha = state->state.alpha,
.zpos = state->state.zpos,
};
unsigned int i;
@@ -351,44 +351,13 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane)
if (state == NULL)
return;

- state->alpha = 255;
+ state->state.alpha = 255;
state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;

plane->state = &state->state;
plane->state->plane = plane;
}

-static int rcar_du_vsp_plane_atomic_set_property(struct drm_plane *plane,
- struct drm_plane_state *state, struct drm_property *property,
- uint64_t val)
-{
- struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
- struct rcar_du_device *rcdu = to_rcar_vsp_plane(plane)->vsp->dev;
-
- if (property == rcdu->props.alpha)
- rstate->alpha = val;
- else
- return -EINVAL;
-
- return 0;
-}
-
-static int rcar_du_vsp_plane_atomic_get_property(struct drm_plane *plane,
- const struct drm_plane_state *state, struct drm_property *property,
- uint64_t *val)
-{
- const struct rcar_du_vsp_plane_state *rstate =
- container_of(state, const struct rcar_du_vsp_plane_state, state);
- struct rcar_du_device *rcdu = to_rcar_vsp_plane(plane)->vsp->dev;
-
- if (property == rcdu->props.alpha)
- *val = rstate->alpha;
- else
- return -EINVAL;
-
- return 0;
-}
-
static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -396,8 +365,6 @@ static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = {
.destroy = drm_plane_cleanup,
.atomic_duplicate_state = rcar_du_vsp_plane_atomic_duplicate_state,
.atomic_destroy_state = rcar_du_vsp_plane_atomic_destroy_state,
- .atomic_set_property = rcar_du_vsp_plane_atomic_set_property,
- .atomic_get_property = rcar_du_vsp_plane_atomic_get_property,
};

int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
@@ -454,8 +421,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
if (type == DRM_PLANE_TYPE_PRIMARY)
continue;

- drm_object_attach_property(&plane->plane.base,
- rcdu->props.alpha, 255);
+ drm_plane_create_alpha_property(&plane->plane, 255);
drm_plane_create_zpos_property(&plane->plane, 1, 1,
vsp->num_planes - 1);
}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index f876c512163c..8b19914761e4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -44,7 +44,6 @@ static inline struct rcar_du_vsp_plane *to_rcar_vsp_plane(struct drm_plane *p)
* @state: base DRM plane state
* @format: information about the pixel format used by the plane
* @sg_tables: scatter-gather tables for the frame buffer memory
- * @alpha: value of the plane alpha property
* @zpos: value of the plane zpos property
*/
struct rcar_du_vsp_plane_state {
@@ -53,7 +52,6 @@ struct rcar_du_vsp_plane_state {
const struct rcar_du_format_info *format;
struct sg_table sg_tables[3];

- unsigned int alpha;
unsigned int zpos;
};

--
git-series 0.9.1

2018-01-22 10:40:30

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 19/19] drm/sun4i: backend: Remove ARGB spoofing

We've had some code for quite some time to prevent the alpha bug from
happening on the lowest primary plane. Since we now check for this in our
atomic_check, we can simply remove it.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_backend.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index f5beeec06fd5..b414dc87e217 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -92,13 +92,8 @@ void sun4i_backend_layer_enable(struct sun4i_backend *backend,
SUN4I_BACKEND_MODCTL_LAY_EN(layer), val);
}

-static int sun4i_backend_drm_format_to_layer(struct drm_plane *plane,
- u32 format, u32 *mode)
+static int sun4i_backend_drm_format_to_layer(u32 format, u32 *mode)
{
- if (plane && (plane->type == DRM_PLANE_TYPE_PRIMARY) &&
- (format == DRM_FORMAT_ARGB8888))
- format = DRM_FORMAT_XRGB8888;
-
switch (format) {
case DRM_FORMAT_ARGB8888:
*mode = SUN4I_BACKEND_LAY_FBFMT_ARGB8888;
@@ -200,8 +195,7 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend *backend,
SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
val);

- ret = sun4i_backend_drm_format_to_layer(plane, fb->format->format,
- &val);
+ ret = sun4i_backend_drm_format_to_layer(fb->format->format, &val);
if (ret) {
DRM_DEBUG_DRIVER("Invalid format\n");
return ret;
@@ -220,7 +214,7 @@ int sun4i_backend_update_layer_frontend(struct sun4i_backend *backend,
u32 val;
int ret;

- ret = sun4i_backend_drm_format_to_layer(NULL, fmt, &val);
+ ret = sun4i_backend_drm_format_to_layer(fmt, &val);
if (ret) {
DRM_DEBUG_DRIVER("Invalid format\n");
return ret;
--
git-series 0.9.1

2018-01-22 10:40:37

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 10/19] drm/sun4i: backend: Fix define typo

There was a typo in the width spelling of the (unused)
SUN4I_BACKEND_IYUVLINEWITDTH_REG macro. Fix it.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_backend.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
index b5edf2d50a24..1ca8b7db6807 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.h
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
@@ -112,7 +112,9 @@
#define SUN4I_BACKEND_SPRALPHACTL_REG 0x90c
#define SUN4I_BACKEND_IYUVCTL_REG 0x920
#define SUN4I_BACKEND_IYUVADD_REG(c) (0x930 + (0x4 * (c)))
-#define SUN4I_BACKEND_IYUVLINEWITDTH_REG(c) (0x940 + (0x4 * (c)))
+
+#define SUN4I_BACKEND_IYUVLINEWIDTH_REG(c) (0x940 + (0x4 * (c)))
+
#define SUN4I_BACKEND_YGCOEF_REG(c) (0x950 + (0x4 * (c)))
#define SUN4I_BACKEND_YGCONS_REG 0x95c
#define SUN4I_BACKEND_URCOEF_REG(c) (0x960 + (0x4 * (c)))
--
git-series 0.9.1

2018-01-22 10:41:36

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 05/19] drm/vc4: Use the alpha format field in drm_format_info

Now that the drm_format_info has a alpha field to tell if a format embeds
an alpha component in it, let's use it.

Cc: Eric Anholt <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/vc4/vc4_plane.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 423a23ed8fc2..a0a626242bcd 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -85,40 +85,39 @@ static const struct hvs_format {
u32 drm; /* DRM_FORMAT_* */
u32 hvs; /* HVS_FORMAT_* */
u32 pixel_order;
- bool has_alpha;
bool flip_cbcr;
} hvs_formats[] = {
{
.drm = DRM_FORMAT_XRGB8888, .hvs = HVS_PIXEL_FORMAT_RGBA8888,
- .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = false,
+ .pixel_order = HVS_PIXEL_ORDER_ABGR,
},
{
.drm = DRM_FORMAT_ARGB8888, .hvs = HVS_PIXEL_FORMAT_RGBA8888,
- .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = true,
+ .pixel_order = HVS_PIXEL_ORDER_ABGR,
},
{
.drm = DRM_FORMAT_ABGR8888, .hvs = HVS_PIXEL_FORMAT_RGBA8888,
- .pixel_order = HVS_PIXEL_ORDER_ARGB, .has_alpha = true,
+ .pixel_order = HVS_PIXEL_ORDER_ARGB,
},
{
.drm = DRM_FORMAT_XBGR8888, .hvs = HVS_PIXEL_FORMAT_RGBA8888,
- .pixel_order = HVS_PIXEL_ORDER_ARGB, .has_alpha = false,
+ .pixel_order = HVS_PIXEL_ORDER_ARGB,
},
{
.drm = DRM_FORMAT_RGB565, .hvs = HVS_PIXEL_FORMAT_RGB565,
- .pixel_order = HVS_PIXEL_ORDER_XRGB, .has_alpha = false,
+ .pixel_order = HVS_PIXEL_ORDER_XRGB,
},
{
.drm = DRM_FORMAT_BGR565, .hvs = HVS_PIXEL_FORMAT_RGB565,
- .pixel_order = HVS_PIXEL_ORDER_XBGR, .has_alpha = false,
+ .pixel_order = HVS_PIXEL_ORDER_XBGR,
},
{
.drm = DRM_FORMAT_ARGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551,
- .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = true,
+ .pixel_order = HVS_PIXEL_ORDER_ABGR,
},
{
.drm = DRM_FORMAT_XRGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551,
- .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = false,
+ .pixel_order = HVS_PIXEL_ORDER_ABGR,
},
{
.drm = DRM_FORMAT_YUV422,
@@ -601,7 +600,7 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
/* Position Word 2: Source Image Size, Alpha Mode */
vc4_state->pos2_offset = vc4_state->dlist_count;
vc4_dlist_write(vc4_state,
- VC4_SET_FIELD(format->has_alpha ?
+ VC4_SET_FIELD(fb->format->has_alpha ?
SCALER_POS2_ALPHA_MODE_PIPELINE :
SCALER_POS2_ALPHA_MODE_FIXED,
SCALER_POS2_ALPHA_MODE) |
--
git-series 0.9.1

2018-01-22 10:42:12

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 01/19] drm/fourcc: Add a alpha field to drm_format_info

There's a bunch of drivers that duplicate the same function to know if a
particular format embeds an alpha component or not.

Let's create a field in the drm_format_info to avoid duplicating that logic
and looking up formats all the time.

Cc: Eric Anholt <[email protected]>
Cc: Inki Dae <[email protected]>
Cc: Joonyoung Shim <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Mark Yao <[email protected]>
Cc: Seung-Woo Kim <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/drm_fourcc.c | 50 ++++++++++++++++++-------------------
include/drm/drm_fourcc.h | 2 +-
2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 9c0152df45ad..5ca6395cd4d3 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -112,18 +112,18 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_XBGR4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_RGBX4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGRX4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ARGB4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ABGR4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGBA4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGRA4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ARGB4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_ABGR4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_RGBA4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGRA4444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_XRGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_XBGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_RGBX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGRX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ARGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ABGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGBA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGRA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ARGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_ABGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_RGBA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGRA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGR565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_RGB888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
@@ -132,26 +132,26 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_RGBX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGRX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGB565_A8, .depth = 24, .num_planes = 2, .cpp = { 2, 1, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGR565_A8, .depth = 24, .num_planes = 2, .cpp = { 2, 1, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_RGB565_A8, .depth = 24, .num_planes = 2, .cpp = { 2, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGR565_A8, .depth = 24, .num_planes = 2, .cpp = { 2, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_RGBX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
{ .format = DRM_FORMAT_BGRX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGBA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGRA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGBA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGRA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGB888_A8, .depth = 32, .num_planes = 2, .cpp = { 3, 1, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGR888_A8, .depth = 32, .num_planes = 2, .cpp = { 3, 1, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_XRGB8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_XBGR8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_RGBX8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1 },
- { .format = DRM_FORMAT_BGRX8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_RGBA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGRA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_RGBA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGRA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_RGB888_A8, .depth = 32, .num_planes = 2, .cpp = { 3, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGR888_A8, .depth = 32, .num_planes = 2, .cpp = { 3, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_XRGB8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_XBGR8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_RGBX8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+ { .format = DRM_FORMAT_BGRX8888_A8, .depth = 32, .num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_YUV410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
{ .format = DRM_FORMAT_YVU410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
{ .format = DRM_FORMAT_YUV411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
@@ -172,7 +172,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
{ .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
{ .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
- { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+ { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
};

unsigned int i;
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 6942e84b6edd..3e86408dac9f 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -38,6 +38,7 @@ struct drm_mode_fb_cmd2;
* @cpp: Number of bytes per pixel (per plane)
* @hsub: Horizontal chroma subsampling factor
* @vsub: Vertical chroma subsampling factor
+ * @has_alpha: Does the format embeds an alpha component?
*/
struct drm_format_info {
u32 format;
@@ -46,6 +47,7 @@ struct drm_format_info {
u8 cpp[3];
u8 hsub;
u8 vsub;
+ bool has_alpha;
};

/**
--
git-series 0.9.1

2018-01-22 10:42:37

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 04/19] drm/rockchip: Use the alpha format field in drm_format_info

Now that the drm_format_info has a alpha field to tell if a format embeds
an alpha component in it, let's use it.

Acked-by: Sandy huang <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 19128b4dea54..db6a0662be5a 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -253,17 +253,6 @@ static bool is_yuv_support(uint32_t format)
}
}

-static bool is_alpha_support(uint32_t format)
-{
- switch (format) {
- case DRM_FORMAT_ARGB8888:
- case DRM_FORMAT_ABGR8888:
- return true;
- default:
- return false;
- }
-}
-
static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
uint32_t dst, bool is_horizontal,
int vsu_mode, int *vskiplines)
@@ -790,7 +779,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
rb_swap = has_rb_swapped(fb->format->format);
VOP_WIN_SET(vop, win, rb_swap, rb_swap);

- if (is_alpha_support(fb->format->format)) {
+ if (fb->format->has_alpha) {
VOP_WIN_SET(vop, win, dst_alpha_ctl,
DST_FACTOR_M0(ALPHA_SRC_INVERSE));
val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
--
git-series 0.9.1

2018-01-22 10:42:39

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 13/19] drm/sun4i: backend: Set a default zpos in our reset hook

The our plane state zpos value will be set only if there's an existing
state attached to the plane when creating the property.

However, this is not the case during the probe, and we therefore need to
put our default value in our reset hook.

Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/sun4i/sun4i_layer.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
index c448cb6b9fa9..03549646528a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_layer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
@@ -28,6 +28,7 @@ struct sun4i_plane_desc {

static void sun4i_backend_layer_reset(struct drm_plane *plane)
{
+ struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
struct sun4i_layer_state *state;

if (plane->state) {
@@ -43,6 +44,7 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
if (state) {
plane->state = &state->state;
plane->state->plane = plane;
+ plane->state->zpos = layer->id;
}
}

--
git-series 0.9.1

2018-01-22 10:42:54

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 03/19] drm/atmel-exynos: Use the alpha format field in drm_format_info

Now that the drm_format_info has a alpha field to tell if a format embeds
an alpha component in it, let's use it.

Cc: Joonyoung Shim <[email protected]>
Cc: Kyungmin Park <[email protected]>
Cc: Seung-Woo Kim <[email protected]>
Acked-by: Inki Dae <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/exynos/exynos_mixer.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index dc5d79465f9b..257299ec95c4 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -179,18 +179,6 @@ static const u8 filter_cr_horiz_tap4[] = {
70, 59, 48, 37, 27, 19, 11, 5,
};

-static inline bool is_alpha_format(unsigned int pixel_format)
-{
- switch (pixel_format) {
- case DRM_FORMAT_ARGB8888:
- case DRM_FORMAT_ARGB1555:
- case DRM_FORMAT_ARGB4444:
- return true;
- default:
- return false;
- }
-}
-
static inline u32 vp_reg_read(struct mixer_context *ctx, u32 reg_id)
{
return readl(ctx->vp_regs + reg_id);
@@ -625,7 +613,7 @@ static void mixer_graph_buffer(struct mixer_context *ctx,
mixer_reg_write(ctx, MXR_GRAPHIC_BASE(win), dma_addr);

mixer_cfg_layer(ctx, win, priority, true);
- mixer_cfg_gfx_blend(ctx, win, is_alpha_format(fb->format->format));
+ mixer_cfg_gfx_blend(ctx, win, fb->format->has_alpha);

/* layer update mandatory for mixer 16.0.33.0 */
if (ctx->mxr_ver == MXR_VER_16_0_33_0 ||
--
git-series 0.9.1

2018-01-22 10:43:04

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 06/19] drm/blend: Add a generic alpha property

Some drivers duplicate the logic to create a property to store a per-plane
alpha.

This is especially useful if we ever want to support extra protocols for
Wayland like:
https://lists.freedesktop.org/archives/wayland-devel/2017-August/034741.html

Let's create a helper in order to move that to the core.

Cc: Laurent Pinchart <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
Documentation/gpu/kms-properties.csv | 2 +-
drivers/gpu/drm/drm_atomic.c | 4 ++++-
drivers/gpu/drm/drm_atomic_helper.c | 4 ++++-
drivers/gpu/drm/drm_blend.c | 32 +++++++++++++++++++++++++++++-
include/drm/drm_blend.h | 1 +-
include/drm/drm_plane.h | 6 +++++-
6 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
index 927b65e14219..25ad3503d663 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -99,5 +99,5 @@ radeon,DVI-I,“coherent”,RANGE,"Min=0, Max=1",Connector,TBD
,,"""underscan vborder""",RANGE,"Min=0, Max=128",Connector,TBD
,Audio,“audio”,ENUM,"{ ""off"", ""on"", ""auto"" }",Connector,TBD
,FMT Dithering,“dither”,ENUM,"{ ""off"", ""on"" }",Connector,TBD
-rcar-du,Generic,"""alpha""",RANGE,"Min=0, Max=255",Plane,TBD
+,,"""alpha""",RANGE,"Min=0, Max=Driver dependant",Plane,Opacity of the plane from transparent (0) to fully opaque (MAX). If this property is set to a value different than max, and that the pixel will define an alpha component, the property will have precendance and the pixel value will be ignored. The alpha value is represented as straight alpha, ie the colors haven't been pre-adjusted for their opacity by multiplication. Therefore, the equation to get a color value for one pixel, assuming two planes A and B, will be (color_a * alpha_a + color_b * alpha_b * (MAX - alpha_a) / MAX) / (alpha_a + alpha_b * (MAX - alpha_a) / MAX)
,,"""colorkey""",RANGE,"Min=0, Max=0x01ffffff",Plane,TBD
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index c2da5585e201..ade18cf62c89 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -749,6 +749,8 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_w = val;
} else if (property == config->prop_src_h) {
state->src_h = val;
+ } else if (property == plane->alpha_property) {
+ state->alpha = val;
} else if (property == plane->rotation_property) {
if (!is_power_of_2(val & DRM_MODE_ROTATE_MASK))
return -EINVAL;
@@ -810,6 +812,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_w;
} else if (property == config->prop_src_h) {
*val = state->src_h;
+ } else if (property == plane->alpha_property) {
+ *val = state->alpha;
} else if (property == plane->rotation_property) {
*val = state->rotation;
} else if (property == plane->zpos_property) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71d712f1b56a..bad0af053384 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3373,6 +3373,10 @@ void drm_atomic_helper_plane_reset(struct drm_plane *plane)
if (plane->state) {
plane->state->plane = plane;
plane->state->rotation = DRM_MODE_ROTATE_0;
+
+ /* Reset the alpha value to fully opaque if it matters */
+ if (plane->alpha_property)
+ plane->state->alpha = plane->alpha_property->values[1];
}
}
EXPORT_SYMBOL(drm_atomic_helper_plane_reset);
diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 2e5e089dd912..e6bfa618e902 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -104,6 +104,38 @@
*/

/**
+ * drm_plane_create_alpha_property - create a new alpha property
+ * @plane: drm plane
+ * @max_alpha: maximum value of alpha
+ *
+ * This function initializes a generic, mutable, alpha property and
+ * enables support for it in the DRM core.
+ *
+ * The alpha property will be allowed to be within the bounds of 0
+ * (transparent) to @max_alpha (opaque)
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_plane_create_alpha_property(struct drm_plane *plane, u16 max_alpha)
+{
+ struct drm_property *prop;
+
+ prop = drm_property_create_range(plane->dev, 0, "alpha", 0, max_alpha);
+ if (!prop)
+ return -ENOMEM;
+
+ drm_object_attach_property(&plane->base, prop, max_alpha);
+ plane->alpha_property = prop;
+
+ if (plane->state)
+ plane->state->alpha = max_alpha;
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_plane_create_alpha_property);
+
+/**
* drm_plane_create_rotation_property - create a new rotation property
* @plane: drm plane
* @rotation: initial value of the rotation property
diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
index 17606026590b..e5affba6ebde 100644
--- a/include/drm/drm_blend.h
+++ b/include/drm/drm_blend.h
@@ -36,6 +36,7 @@ static inline bool drm_rotation_90_or_270(unsigned int rotation)
return rotation & (DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270);
}

+int drm_plane_create_alpha_property(struct drm_plane *plane, u16 alpha);
int drm_plane_create_rotation_property(struct drm_plane *plane,
unsigned int rotation,
unsigned int supported_rotations);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 571615079230..a5e26064b132 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -42,6 +42,7 @@ struct drm_modeset_acquire_ctx;
* plane (in 16.16)
* @src_w: width of visible portion of plane (in 16.16)
* @src_h: height of visible portion of plane (in 16.16)
+ * @alpha: opacity of the plane
* @rotation: rotation of the plane
* @zpos: priority of the given plane on crtc (optional)
* Note that multiple active planes on the same crtc can have an identical
@@ -105,6 +106,9 @@ struct drm_plane_state {
uint32_t src_x, src_y;
uint32_t src_h, src_w;

+ /* Plane opacity */
+ u8 alpha;
+
/* Plane rotation */
unsigned int rotation;

@@ -481,6 +485,7 @@ enum drm_plane_type {
* @funcs: helper functions
* @properties: property tracking for this plane
* @type: type of plane (overlay, primary, cursor)
+ * @alpha_property: alpha property for this plane
* @zpos_property: zpos property for this plane
* @rotation_property: rotation property for this plane
* @helper_private: mid-layer private data
@@ -546,6 +551,7 @@ struct drm_plane {
*/
struct drm_plane_state *state;

+ struct drm_property *alpha_property;
struct drm_property *zpos_property;
struct drm_property *rotation_property;
};
--
git-series 0.9.1

2018-01-22 10:43:57

by Maxime Ripard

[permalink] [raw]
Subject: [PATCH v2 02/19] drm/atmel-hlcdc: Use the alpha format field in drm_format_info

Now that the drm_format_info has a alpha field to tell if a format embeds
an alpha component in it, let's use it.

Acked-by: Boris Brezillon <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 24 +++---------------
1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 703c2d13603f..e18800ed7cd1 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -194,20 +194,6 @@ static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode)
return 0;
}

-static bool atmel_hlcdc_format_embeds_alpha(u32 format)
-{
- int i;
-
- for (i = 0; i < sizeof(format); i++) {
- char tmp = (format >> (8 * i)) & 0xff;
-
- if (tmp == 'A')
- return true;
- }
-
- return false;
-}
-
static u32 heo_downscaling_xcoef[] = {
0x11343311,
0x000000f7,
@@ -377,13 +363,13 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
{
unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
- u32 format = state->base.fb->format->format;
+ const struct drm_format_info *format = state->base.fb->format;

/*
* Rotation optimization is not working on RGB888 (rotation is still
* working but without any optimization).
*/
- if (format == DRM_FORMAT_RGB888)
+ if (format->format == DRM_FORMAT_RGB888)
cfg |= ATMEL_HLCDC_LAYER_DMA_ROTDIS;

atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_HLCDC_LAYER_DMA_CFG,
@@ -395,7 +381,7 @@ atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
cfg |= ATMEL_HLCDC_LAYER_OVR | ATMEL_HLCDC_LAYER_ITER2BL |
ATMEL_HLCDC_LAYER_ITER;

- if (atmel_hlcdc_format_embeds_alpha(format))
+ if (format->has_alpha)
cfg |= ATMEL_HLCDC_LAYER_LAEN;
else
cfg |= ATMEL_HLCDC_LAYER_GAEN |
@@ -566,7 +552,7 @@ atmel_hlcdc_plane_prepare_disc_area(struct drm_crtc_state *c_state)
ovl_state = drm_plane_state_to_atmel_hlcdc_plane_state(ovl_s);

if (!ovl_s->fb ||
- atmel_hlcdc_format_embeds_alpha(ovl_s->fb->format->format) ||
+ ovl_s->fb->format->has_alpha ||
ovl_state->alpha != 255)
continue;

@@ -769,7 +755,7 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,

if ((state->crtc_h != state->src_h || state->crtc_w != state->src_w) &&
(!desc->layout.memsize ||
- atmel_hlcdc_format_embeds_alpha(state->base.fb->format->format)))
+ state->base.fb->format->has_alpha))
return -EINVAL;

if (state->crtc_x < 0 || state->crtc_y < 0)
--
git-series 0.9.1

2018-01-22 20:35:52

by Eric Anholt

[permalink] [raw]
Subject: Re: [PATCH v2 05/19] drm/vc4: Use the alpha format field in drm_format_info

Maxime Ripard <[email protected]> writes:

> Now that the drm_format_info has a alpha field to tell if a format embeds
> an alpha component in it, let's use it.
>
> Cc: Eric Anholt <[email protected]>
> Reviewed-by: Daniel Vetter <[email protected]>
> Signed-off-by: Maxime Ripard <[email protected]>

I had sent my r-b for the last version of this.


Attachments:
signature.asc (847.00 B)

2018-01-29 03:02:46

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 09/19] drm/sun4i: backend: Fix structure indentation

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> The sun4i_plane_desc structure was somehow indented to two tabulations
> instead of one as we shoud do. Fix that.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Acked-by: Chen-Yu Tsai <[email protected]>

2018-01-29 03:02:57

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 12/19] drm/sun4i: backend: Move the coord function in the shared part

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> The function supposed to update a plane's coordinates is called in both
> branches of our function. Let's move it out the if statement.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Acked-by: Chen-Yu Tsai <[email protected]>

2018-01-29 03:03:01

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 13/19] drm/sun4i: backend: Set a default zpos in our reset hook

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> The our plane state zpos value will be set only if there's an existing

^^^ extra word

Otherwise,

Acked-by: Chen-Yu Tsai <[email protected]>

> state attached to the plane when creating the property.
>
> However, this is not the case during the probe, and we therefore need to
> put our default value in our reset hook.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> drivers/gpu/drm/sun4i/sun4i_layer.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index c448cb6b9fa9..03549646528a 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -28,6 +28,7 @@ struct sun4i_plane_desc {
>
> static void sun4i_backend_layer_reset(struct drm_plane *plane)
> {
> + struct sun4i_layer *layer = plane_to_sun4i_layer(plane);
> struct sun4i_layer_state *state;
>
> if (plane->state) {
> @@ -43,6 +44,7 @@ static void sun4i_backend_layer_reset(struct drm_plane *plane)
> if (state) {
> plane->state = &state->state;
> plane->state->plane = plane;
> + plane->state->zpos = layer->id;
> }
> }
>
> --
> git-series 0.9.1

2018-01-29 03:03:20

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 15/19] drm/sun4i: backend: Check for the number of alpha planes

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> Due to the way the composition is done in hardware, we can only have a
> single alpha-enabled plane active at a time, placed in the second (highest
> priority) pipe.
>
> Make sure of that in our atomic_check to not end up in an impossible
> scenario.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 50 ++++++++++++++++++++++++++++-
> drivers/gpu/drm/sun4i/sun4i_backend.h | 2 +-
> drivers/gpu/drm/sun4i/sun4i_layer.c | 23 +-------------
> 3 files changed, 53 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index c4986054909b..eb1749d2c0d5 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -329,6 +329,8 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *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;
>
> DRM_DEBUG_DRIVER("Starting checking our planes\n");
> @@ -341,6 +343,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> drm_atomic_get_plane_state(state, plane);
> struct sun4i_layer_state *layer_state =
> state_to_sun4i_layer_state(plane_state);
> + struct drm_framebuffer *fb = plane_state->fb;
>
> if (sun4i_backend_plane_uses_frontend(plane_state)) {
> DRM_DEBUG_DRIVER("Using the frontend for plane %d\n",
> @@ -351,6 +354,50 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> } else {
> layer_state->uses_frontend = false;
> }
> +
> + DRM_DEBUG_DRIVER("Plane FB format is %s\n",
> + drm_get_format_name(fb->format->format,
> + &format_name));
> + if (fb->format->has_alpha)
> + num_alpha_planes++;
> +
> + num_planes++;
> + }
> +
> + /*
> + * 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, 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).
> + *
> + * Since means that we effectively have only three valid

^^^^^ This

> + * 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) {
> + DRM_DEBUG_DRIVER("Too many planes with alpha, rejecting...\n");
> + return -EINVAL;
> }
>
> if (num_frontend_planes > SUN4I_BACKEND_NUM_FRONTEND_LAYERS) {
> @@ -358,6 +405,9 @@ static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> return -EINVAL;
> }
>
> + DRM_DEBUG_DRIVER("State valid with %u planes, %u alpha, %u video\n",
> + num_planes, num_alpha_planes, num_frontend_planes);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 04a4f11b87a8..52e77591186a 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -146,6 +146,8 @@
> #define SUN4I_BACKEND_HWCCOLORTAB_OFF 0x4c00
> #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
>
> struct sun4i_backend {
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index fbf25d59cf88..900e716443b8 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -201,32 +201,11 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
> struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
> int i;
>
> - planes = devm_kcalloc(drm->dev, ARRAY_SIZE(sun4i_backend_planes) + 1,
> + planes = devm_kcalloc(drm->dev, SUN4I_BACKEND_NUM_LAYERS,
> sizeof(*planes), GFP_KERNEL);

The returned "planes" array has to have a sentinel at the end, as we
never return the actual number of layers created. That is what the +1
was for in the original code.

Otherwise,

Reviewed-by: Chen-Yu Tsai <[email protected]>

> if (!planes)
> return ERR_PTR(-ENOMEM);
>
> - /*
> - * 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;
> --
> git-series 0.9.1

2018-01-29 03:03:24

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 16/19] drm/sun4i: backend: Assign the pipes automatically

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> Since we now have a way to enforce the zpos, check for the number of alpha
> planes, the only missing part is to assign our pipe automatically instead
> of hardcoding it.
>
> The algorithm is quite simple, but requires two iterations over the list of
> planes.
>
> In the first one (which is the same one that we've had to check for alpha,
> the frontend usage, and so on), we order the planes by their zpos.
>
> We can then do a second iteration over that array by ascending zpos
> starting with the pipe 0. When and if we encounter our alpha plane, we put
> it and all the other subsequent planes in the second pipe.
>
> And since we have runtime checks and pipe assignments now, we can just
> remove the static declaration of the planes we used to have.

It would be slightly better if you could split this out into a separate
patch. It's not immediately obvious to others that the changes to
sun4i_layer.c are self-contained, while the change to sun4i_layer.h
is part of the new pipeline tracking thing. Plus I think the way you
structured everything means it won't break if you split it out.

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-01-29 03:03:26

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 11/19] drm/sun4i: framebuffer: Add a custom atomic_check

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> In order to support normalized zpos, we need to call
> drm_atomic_normalize_zpos in our driver's drm_mode_config_funcs'
> atomic_check.
>
> Let's duplicate the definition of drm_atomic_helper_check for now.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Acked-by: Chen-Yu Tsai <[email protected]>

2018-01-29 03:04:00

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 17/19] drm/sun4i: backend: Make zpos configurable

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> Now that we have everything in place, we can make zpos configurable now.
> Change the zpos property from an immutable one to a regular.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-01-29 03:07:48

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 10/19] drm/sun4i: backend: Fix define typo

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> There was a typo in the width spelling of the (unused)
> SUN4I_BACKEND_IYUVLINEWITDTH_REG macro. Fix it.
>
> Signed-off-by: Maxime Ripard <[email protected]>

Acked-by: Chen-Yu Tsai <[email protected]>

2018-01-29 03:07:48

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v2 14/19] drm/sun4i: backend: Add support for zpos

On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
<[email protected]> wrote:
> Our various planes have a configurable zpos, that combined with the pipes
> allow to configure the composition.
>
> Since the interaction between the pipes, zpos and alphas framebuffers are

^^^ is

> not trivials, let's just enable the zpos as an immutable property for now,

^^^^^^^ trivial

> and use that zpos in our atomic_update part.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> ---
> drivers/gpu/drm/sun4i/sun4i_backend.c | 15 +++++++++++++++
> drivers/gpu/drm/sun4i/sun4i_backend.h | 2 ++
> drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 4 ++++
> drivers/gpu/drm/sun4i/sun4i_layer.c | 3 +++
> 4 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index a18c86a15748..c4986054909b 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -272,6 +272,21 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> return 0;
> }
>
> +int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend, int layer,
> + struct drm_plane *plane)
> +{
> + struct drm_plane_state *state = plane->state;
> + unsigned int priority = state->normalized_zpos;
> +
> + DRM_DEBUG_DRIVER("Setting layer %d priority to %d\n", layer, priority);

You might want to make the statement less ambiguous, like

"Setting layer %d's priority ..."

Otherwise,

Reviewed-by: Chen-Yu Tsai <[email protected]>


> +
> + regmap_update_bits(backend->engine.regs, SUN4I_BACKEND_ATTCTL_REG0(layer),
> + SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL_MASK,
> + SUN4I_BACKEND_ATTCTL_REG0_LAY_PRISEL(priority));
> +
> + return 0;
> +}
> +
> static bool sun4i_backend_plane_uses_scaler(struct drm_plane_state *state)
> {
> u16 src_h = state->src_h >> 16;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.h b/drivers/gpu/drm/sun4i/sun4i_backend.h
> index 1ca8b7db6807..04a4f11b87a8 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.h
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.h
> @@ -182,5 +182,7 @@ 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);
> +int sun4i_backend_update_layer_zpos(struct sun4i_backend *backend,
> + int layer, struct drm_plane *plane);
>
> #endif /* _SUN4I_BACKEND_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
> index e68004844abe..5b3986437a50 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
> @@ -35,6 +35,10 @@ static int sun4i_de_atomic_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> + ret = drm_atomic_normalize_zpos(dev, state);
> + if (ret)
> + return ret;
> +
> return drm_atomic_helper_check_planes(dev, state);
> }
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.c b/drivers/gpu/drm/sun4i/sun4i_layer.c
> index 03549646528a..fbf25d59cf88 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.c
> @@ -115,6 +115,7 @@ static void sun4i_backend_layer_atomic_update(struct drm_plane *plane,
> }
>
> sun4i_backend_update_layer_coord(backend, layer->id, plane);
> + sun4i_backend_update_layer_zpos(backend, layer->id, plane);
> sun4i_backend_layer_enable(backend, layer->id, true);
> }
>
> @@ -237,6 +238,8 @@ struct drm_plane **sun4i_layers_init(struct drm_device *drm,
> return ERR_CAST(layer);
> };
>
> + drm_plane_create_zpos_immutable_property(&layer->plane, i);
> +
> 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),
> --
> git-series 0.9.1

2018-01-29 13:07:51

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 05/19] drm/vc4: Use the alpha format field in drm_format_info

Hi Eric,

On Tue, Jan 23, 2018 at 07:35:10AM +1100, Eric Anholt wrote:
> Maxime Ripard <[email protected]> writes:
>
> > Now that the drm_format_info has a alpha field to tell if a format embeds
> > an alpha component in it, let's use it.
> >
> > Cc: Eric Anholt <[email protected]>
> > Reviewed-by: Daniel Vetter <[email protected]>
> > Signed-off-by: Maxime Ripard <[email protected]>
>
> I had sent my r-b for the last version of this.

Sorry, I forgot to carry it :/

Thanks for your review, I pushed the patches 1-5 to drm-misc

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (700.00 B)
signature.asc (849.00 B)
Download all attachments

2018-01-29 13:09:19

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 16/19] drm/sun4i: backend: Assign the pipes automatically

On Mon, Jan 29, 2018 at 10:22:54AM +0800, Chen-Yu Tsai wrote:
> On Mon, Jan 22, 2018 at 6:35 PM, Maxime Ripard
> <[email protected]> wrote:
> > Since we now have a way to enforce the zpos, check for the number of alpha
> > planes, the only missing part is to assign our pipe automatically instead
> > of hardcoding it.
> >
> > The algorithm is quite simple, but requires two iterations over the list of
> > planes.
> >
> > In the first one (which is the same one that we've had to check for alpha,
> > the frontend usage, and so on), we order the planes by their zpos.
> >
> > We can then do a second iteration over that array by ascending zpos
> > starting with the pipe 0. When and if we encounter our alpha plane, we put
> > it and all the other subsequent planes in the second pipe.
> >
> > And since we have runtime checks and pipe assignments now, we can just
> > remove the static declaration of the planes we used to have.
>
> It would be slightly better if you could split this out into a separate
> patch. It's not immediately obvious to others that the changes to
> sun4i_layer.c are self-contained, while the change to sun4i_layer.h
> is part of the new pipeline tracking thing. Plus I think the way you
> structured everything means it won't break if you split it out.

You're right, I'll split this patch and resend it. I've pushed the
patches 9-15 to drm-misc, with your suggestions when you had some.

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (1.55 kB)
signature.asc (849.00 B)
Download all attachments