436 lines
15 KiB
Diff
436 lines
15 KiB
Diff
From d0103b81264af511b074a0627a64c607fe0427c8 Mon Sep 17 00:00:00 2001
|
|
From: Ondrej Jirman <megous@megous.com>
|
|
Date: Sun, 8 Sep 2019 11:42:29 +0200
|
|
Subject: [PATCH 312/478] drm: sun8i-ui/vi: Fix layer zpos change/atomic
|
|
modesetting
|
|
|
|
A problem was found where identical configuration of planes leads
|
|
to different register settings at the HW layer when using a X server
|
|
with modesetting driver and one plane marked as a cursor.
|
|
|
|
On first run of the X server, only the black screen and the layer
|
|
containing the cursor is visible. Switching to console and back
|
|
corrects the situation.
|
|
|
|
I have dumped registers, and found out this:
|
|
|
|
1) First Xorg run:
|
|
|
|
0x01101000 : 00000201
|
|
0x01101080 : 00000030
|
|
|
|
BLD_FILL_COLOR_CTL: (aka SUN8I_MIXER_BLEND_PIPE_CTL)
|
|
P1_EN
|
|
|
|
BLD_CH_RTCTL: (aka SUN8I_MIXER_BLEND_ROUTE)
|
|
P0_RTCTL channel0
|
|
P1_RTCTL channel3
|
|
|
|
2) After switch to console and back to Xorg:
|
|
|
|
0x01101000 : 00000301
|
|
0x01101080 : 00000031
|
|
|
|
BLD_FILL_COLOR_CTL:
|
|
P1_EN and P0_EN
|
|
|
|
BLD_CH_RTCTL:
|
|
P0_RTCTL channel1
|
|
P1_RTCTL channel3
|
|
|
|
What happens is that sun8i_ui_layer_enable() function may disable
|
|
blending pipes even if it is no longer assigned to its layer, because
|
|
of incorrect state/zpos tracking in the driver.
|
|
|
|
In particular, layer 1 is configured to zpos 0 and thus uses pipe 0.
|
|
When layer 3 is enabledb y X server, sun8i_ui_layer_enable() will get
|
|
called with old_zpos=0 zpos=1, which will lead to disabling of pipe 0.
|
|
|
|
In general this issue can happen to any layer during enable or zpos
|
|
changes on multiple layers at once.
|
|
|
|
To correct this we now pass previous enabled/disabled state of the
|
|
layer, and pass real previous zpos of the layer to
|
|
sun8i_ui_layer_enable() and rework the sun8i_ui_layer_enable() function
|
|
to take react to the state changes correctly. In order to not complicate
|
|
the atomic_disable callback with all of the above changes, we simply
|
|
remove it and implement all the chanes as part of atomic_update, which
|
|
also reduces the code duplication.
|
|
|
|
To make this all work, initial zpos positions of all layers need to be
|
|
restored to initial values on reset.
|
|
|
|
Signed-off-by: Ondrej Jirman <megous@megous.com>
|
|
---
|
|
drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 120 +++++++++++++++----------
|
|
drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 120 +++++++++++++++----------
|
|
2 files changed, 148 insertions(+), 92 deletions(-)
|
|
|
|
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
|
|
index 7845c2a53a7f..9c406f3896f6 100644
|
|
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
|
|
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
|
|
@@ -24,10 +24,11 @@
|
|
#include "sun8i_ui_scaler.h"
|
|
|
|
static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
|
|
- int overlay, bool enable, unsigned int zpos,
|
|
- unsigned int old_zpos)
|
|
+ int overlay, bool was_enabled, bool enable,
|
|
+ unsigned int zpos, unsigned int old_zpos)
|
|
{
|
|
u32 val, bld_base, ch_base;
|
|
+ unsigned int old_pipe_ch;
|
|
|
|
bld_base = sun8i_blender_base(mixer);
|
|
ch_base = sun8i_channel_base(mixer, channel);
|
|
@@ -35,28 +36,57 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
|
|
DRM_DEBUG_DRIVER("%sabling channel %d overlay %d\n",
|
|
enable ? "En" : "Dis", channel, overlay);
|
|
|
|
- if (enable)
|
|
- val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN;
|
|
- else
|
|
- val = 0;
|
|
+ if (!was_enabled != !enable) {
|
|
+ val = enable ? SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN : 0;
|
|
|
|
- regmap_update_bits(mixer->engine.regs,
|
|
- SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
|
|
- SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
|
|
-
|
|
- if (!enable || zpos != old_zpos) {
|
|
regmap_update_bits(mixer->engine.regs,
|
|
- SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
|
|
- SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
|
|
- 0);
|
|
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay),
|
|
+ SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
|
|
+ }
|
|
|
|
- regmap_update_bits(mixer->engine.regs,
|
|
+ /*
|
|
+ * If this layer was enabled and is being disabled or if it is
|
|
+ * enabled and just changing zpos, clear the old route, if it is
|
|
+ * still configured to this layer in HW.
|
|
+ */
|
|
+ if ((was_enabled && !enable) || (enable && zpos != old_zpos)) {
|
|
+ /* get channel the pipe for old_zpos is routed to from the HW */
|
|
+ regmap_read(mixer->engine.regs,
|
|
SUN8I_MIXER_BLEND_ROUTE(bld_base),
|
|
- SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
|
|
- 0);
|
|
+ &old_pipe_ch);
|
|
+ old_pipe_ch &= SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos);
|
|
+ old_pipe_ch >>= SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(old_zpos);
|
|
+
|
|
+ /*
|
|
+ * Check that pipe for old_zpos is still routed to our layer,
|
|
+ * and clear/disable it if it is.
|
|
+ */
|
|
+
|
|
+ if (old_pipe_ch == channel) {
|
|
+ DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
|
|
+ channel, was_enabled, enable, old_zpos, zpos);
|
|
+
|
|
+ DRM_DEBUG_DRIVER(" disable pipe %d\n", old_zpos);
|
|
+
|
|
+ regmap_update_bits(mixer->engine.regs,
|
|
+ SUN8I_MIXER_BLEND_ROUTE(bld_base),
|
|
+ SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
|
|
+ 0);
|
|
+
|
|
+ regmap_update_bits(mixer->engine.regs,
|
|
+ SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
|
|
+ SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
|
|
+ 0);
|
|
+ }
|
|
}
|
|
|
|
- if (enable) {
|
|
+ /*
|
|
+ * If enabling this layer or changin zpos, set route to this layer.
|
|
+ */
|
|
+ if ((enable && !was_enabled) || (enable && zpos != old_zpos)) {
|
|
+ DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
|
|
+ channel, was_enabled, enable, old_zpos, zpos);
|
|
+
|
|
val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
|
|
|
|
regmap_update_bits(mixer->engine.regs,
|
|
@@ -69,6 +99,8 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
|
|
SUN8I_MIXER_BLEND_ROUTE(bld_base),
|
|
SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos),
|
|
val);
|
|
+
|
|
+ DRM_DEBUG_DRIVER(" enable pipe %d <- ch %d\n", zpos, channel);
|
|
}
|
|
}
|
|
|
|
@@ -288,19 +320,6 @@ static int sun8i_ui_layer_atomic_check(struct drm_plane *plane,
|
|
true, true);
|
|
}
|
|
|
|
-static void sun8i_ui_layer_atomic_disable(struct drm_plane *plane,
|
|
- struct drm_atomic_state *state)
|
|
-{
|
|
- struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
|
|
- plane);
|
|
- struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
|
|
- unsigned int old_zpos = old_state->normalized_zpos;
|
|
- struct sun8i_mixer *mixer = layer->mixer;
|
|
-
|
|
- sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
|
|
- old_zpos);
|
|
-}
|
|
-
|
|
static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
|
|
struct drm_atomic_state *state)
|
|
{
|
|
@@ -312,28 +331,37 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
|
|
unsigned int zpos = new_state->normalized_zpos;
|
|
unsigned int old_zpos = old_state->normalized_zpos;
|
|
struct sun8i_mixer *mixer = layer->mixer;
|
|
+ bool was_enabled = old_state->crtc && old_state->visible;
|
|
+ bool enable = new_state->crtc && new_state->visible;
|
|
|
|
- if (!new_state->visible) {
|
|
- sun8i_ui_layer_enable(mixer, layer->channel,
|
|
- layer->overlay, false, 0, old_zpos);
|
|
- return;
|
|
+ if (enable) {
|
|
+ sun8i_ui_layer_update_coord(mixer, layer->channel,
|
|
+ layer->overlay, plane, zpos);
|
|
+ sun8i_ui_layer_update_alpha(mixer, layer->channel,
|
|
+ layer->overlay, plane);
|
|
+ sun8i_ui_layer_update_formats(mixer, layer->channel,
|
|
+ layer->overlay, plane);
|
|
+ sun8i_ui_layer_update_buffer(mixer, layer->channel,
|
|
+ layer->overlay, plane);
|
|
}
|
|
|
|
- sun8i_ui_layer_update_coord(mixer, layer->channel,
|
|
- layer->overlay, plane, zpos);
|
|
- sun8i_ui_layer_update_alpha(mixer, layer->channel,
|
|
- layer->overlay, plane);
|
|
- sun8i_ui_layer_update_formats(mixer, layer->channel,
|
|
- layer->overlay, plane);
|
|
- sun8i_ui_layer_update_buffer(mixer, layer->channel,
|
|
- layer->overlay, plane);
|
|
sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay,
|
|
- true, zpos, old_zpos);
|
|
+ was_enabled, enable, zpos, old_zpos);
|
|
+}
|
|
+
|
|
+void sun8i_ui_layer_plane_reset(struct drm_plane *plane)
|
|
+{
|
|
+ struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
|
|
+
|
|
+ drm_atomic_helper_plane_reset(plane);
|
|
+ if (!plane->state)
|
|
+ return;
|
|
+
|
|
+ plane->state->zpos = layer->channel;
|
|
}
|
|
|
|
static const struct drm_plane_helper_funcs sun8i_ui_layer_helper_funcs = {
|
|
.atomic_check = sun8i_ui_layer_atomic_check,
|
|
- .atomic_disable = sun8i_ui_layer_atomic_disable,
|
|
.atomic_update = sun8i_ui_layer_atomic_update,
|
|
};
|
|
|
|
@@ -342,7 +370,7 @@ static const struct drm_plane_funcs sun8i_ui_layer_funcs = {
|
|
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
|
|
.destroy = drm_plane_cleanup,
|
|
.disable_plane = drm_atomic_helper_disable_plane,
|
|
- .reset = drm_atomic_helper_plane_reset,
|
|
+ .reset = sun8i_ui_layer_plane_reset,
|
|
.update_plane = drm_atomic_helper_update_plane,
|
|
};
|
|
|
|
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
|
|
index bb7c43036dfa..3929b0e38a10 100644
|
|
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
|
|
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
|
|
@@ -18,10 +18,11 @@
|
|
#include "sun8i_vi_scaler.h"
|
|
|
|
static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
|
|
- int overlay, bool enable, unsigned int zpos,
|
|
- unsigned int old_zpos)
|
|
+ int overlay, bool was_enabled, bool enable,
|
|
+ unsigned int zpos, unsigned int old_zpos)
|
|
{
|
|
u32 val, bld_base, ch_base;
|
|
+ unsigned int old_pipe_ch;
|
|
|
|
bld_base = sun8i_blender_base(mixer);
|
|
ch_base = sun8i_channel_base(mixer, channel);
|
|
@@ -29,28 +30,57 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
|
|
DRM_DEBUG_DRIVER("%sabling VI channel %d overlay %d\n",
|
|
enable ? "En" : "Dis", channel, overlay);
|
|
|
|
- if (enable)
|
|
- val = SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN;
|
|
- else
|
|
- val = 0;
|
|
-
|
|
- regmap_update_bits(mixer->engine.regs,
|
|
- SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
|
|
- SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN, val);
|
|
+ if (!was_enabled != !enable) {
|
|
+ val = enable ? SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN : 0;
|
|
|
|
- if (!enable || zpos != old_zpos) {
|
|
regmap_update_bits(mixer->engine.regs,
|
|
- SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
|
|
- SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
|
|
- 0);
|
|
+ SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay),
|
|
+ SUN8I_MIXER_CHAN_VI_LAYER_ATTR_EN, val);
|
|
+ }
|
|
|
|
- regmap_update_bits(mixer->engine.regs,
|
|
+ /*
|
|
+ * If this layer was enabled and is being disabled or if it is
|
|
+ * enabled and just changing zpos, clear the old route, if it is
|
|
+ * still configured to this layer in HW.
|
|
+ */
|
|
+ if ((was_enabled && !enable) || (enable && zpos != old_zpos)) {
|
|
+ /* get channel the pipe for old_zpos is routed to from the HW */
|
|
+ regmap_read(mixer->engine.regs,
|
|
SUN8I_MIXER_BLEND_ROUTE(bld_base),
|
|
- SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
|
|
- 0);
|
|
+ &old_pipe_ch);
|
|
+ old_pipe_ch &= SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos);
|
|
+ old_pipe_ch >>= SUN8I_MIXER_BLEND_ROUTE_PIPE_SHIFT(old_zpos);
|
|
+
|
|
+ /*
|
|
+ * Check that pipe for old_zpos is still routed to our layer,
|
|
+ * and clear/disable it if it is.
|
|
+ */
|
|
+
|
|
+ if (old_pipe_ch == channel) {
|
|
+ DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
|
|
+ channel, was_enabled, enable, old_zpos, zpos);
|
|
+
|
|
+ DRM_DEBUG_DRIVER(" disable pipe %d\n", old_zpos);
|
|
+
|
|
+ regmap_update_bits(mixer->engine.regs,
|
|
+ SUN8I_MIXER_BLEND_ROUTE(bld_base),
|
|
+ SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
|
|
+ 0);
|
|
+
|
|
+ regmap_update_bits(mixer->engine.regs,
|
|
+ SUN8I_MIXER_BLEND_PIPE_CTL(bld_base),
|
|
+ SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
|
|
+ 0);
|
|
+ }
|
|
}
|
|
|
|
- if (enable) {
|
|
+ /*
|
|
+ * If enabling this layer or changin zpos, set route to this layer.
|
|
+ */
|
|
+ if ((enable && !was_enabled) || (enable && zpos != old_zpos)) {
|
|
+ DRM_DEBUG_DRIVER("chan=%d en=%d->%d zpos=%d->%d\n",
|
|
+ channel, was_enabled, enable, old_zpos, zpos);
|
|
+
|
|
val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
|
|
|
|
regmap_update_bits(mixer->engine.regs,
|
|
@@ -63,6 +93,8 @@ static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
|
|
SUN8I_MIXER_BLEND_ROUTE(bld_base),
|
|
SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(zpos),
|
|
val);
|
|
+
|
|
+ DRM_DEBUG_DRIVER(" enable pipe %d <- ch %d\n", zpos, channel);
|
|
}
|
|
}
|
|
|
|
@@ -392,19 +424,6 @@ static int sun8i_vi_layer_atomic_check(struct drm_plane *plane,
|
|
true, true);
|
|
}
|
|
|
|
-static void sun8i_vi_layer_atomic_disable(struct drm_plane *plane,
|
|
- struct drm_atomic_state *state)
|
|
-{
|
|
- struct drm_plane_state *old_state = drm_atomic_get_old_plane_state(state,
|
|
- plane);
|
|
- struct sun8i_vi_layer *layer = plane_to_sun8i_vi_layer(plane);
|
|
- unsigned int old_zpos = old_state->normalized_zpos;
|
|
- struct sun8i_mixer *mixer = layer->mixer;
|
|
-
|
|
- sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
|
|
- old_zpos);
|
|
-}
|
|
-
|
|
static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
|
|
struct drm_atomic_state *state)
|
|
{
|
|
@@ -416,28 +435,37 @@ static void sun8i_vi_layer_atomic_update(struct drm_plane *plane,
|
|
unsigned int zpos = new_state->normalized_zpos;
|
|
unsigned int old_zpos = old_state->normalized_zpos;
|
|
struct sun8i_mixer *mixer = layer->mixer;
|
|
+ bool was_enabled = old_state->crtc && old_state->visible;
|
|
+ bool enable = new_state->crtc && new_state->visible;
|
|
|
|
- if (!new_state->visible) {
|
|
- sun8i_vi_layer_enable(mixer, layer->channel,
|
|
- layer->overlay, false, 0, old_zpos);
|
|
- return;
|
|
+ if (enable) {
|
|
+ sun8i_vi_layer_update_coord(mixer, layer->channel,
|
|
+ layer->overlay, plane, zpos);
|
|
+ sun8i_vi_layer_update_alpha(mixer, layer->channel,
|
|
+ layer->overlay, plane);
|
|
+ sun8i_vi_layer_update_formats(mixer, layer->channel,
|
|
+ layer->overlay, plane);
|
|
+ sun8i_vi_layer_update_buffer(mixer, layer->channel,
|
|
+ layer->overlay, plane);
|
|
}
|
|
|
|
- sun8i_vi_layer_update_coord(mixer, layer->channel,
|
|
- layer->overlay, plane, zpos);
|
|
- sun8i_vi_layer_update_alpha(mixer, layer->channel,
|
|
- layer->overlay, plane);
|
|
- sun8i_vi_layer_update_formats(mixer, layer->channel,
|
|
- layer->overlay, plane);
|
|
- sun8i_vi_layer_update_buffer(mixer, layer->channel,
|
|
- layer->overlay, plane);
|
|
sun8i_vi_layer_enable(mixer, layer->channel, layer->overlay,
|
|
- true, zpos, old_zpos);
|
|
+ was_enabled, enable, zpos, old_zpos);
|
|
+}
|
|
+
|
|
+void sun8i_vi_layer_plane_reset(struct drm_plane *plane)
|
|
+{
|
|
+ struct sun8i_vi_layer *layer = plane_to_sun8i_vi_layer(plane);
|
|
+
|
|
+ drm_atomic_helper_plane_reset(plane);
|
|
+ if (!plane->state)
|
|
+ return;
|
|
+
|
|
+ plane->state->zpos = layer->channel;
|
|
}
|
|
|
|
static const struct drm_plane_helper_funcs sun8i_vi_layer_helper_funcs = {
|
|
.atomic_check = sun8i_vi_layer_atomic_check,
|
|
- .atomic_disable = sun8i_vi_layer_atomic_disable,
|
|
.atomic_update = sun8i_vi_layer_atomic_update,
|
|
};
|
|
|
|
@@ -446,7 +474,7 @@ static const struct drm_plane_funcs sun8i_vi_layer_funcs = {
|
|
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
|
|
.destroy = drm_plane_cleanup,
|
|
.disable_plane = drm_atomic_helper_disable_plane,
|
|
- .reset = drm_atomic_helper_plane_reset,
|
|
+ .reset = sun8i_vi_layer_plane_reset,
|
|
.update_plane = drm_atomic_helper_update_plane,
|
|
};
|
|
|
|
--
|
|
2.35.3
|
|
|