1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
|
From b8576a2773e4fdc8aed1dfe6ae0954ba53245da1 Mon Sep 17 00:00:00 2001
From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Date: Fri, 5 Apr 2019 09:35:14 -0400
Subject: [PATCH 1824/2940] drm/amd/display: Refactor CRTC interrupt toggling
logic
[Why]
The vblank and pageflip interrupts should only be enabled for a CRTC
that's enabled and has active planes.
The current logic takes care of this, but isn't setup to handle the case
where the active plane count goes to zero but the stream remains
enabled.
We currently block this case since we don't allow commits that enable a
CRTC with no active planes, but shouldn't be any reason we can't support
this from a hardware perspective and many userspace applications expect
to be able to do it (like IGT).
[How]
The count_crtc_active_planes function fills in the number of
"active_planes" on the dm_crtc_state. This should be the same as
DC's plane_count on the stream_status but easier to access since we
don't need to lock the private atomic state with the DC context.
Add the "interrupts_enabled" flag to the dm_crtc_state and set it based
on whether the stream exists and if there are active planes on the
stream.
Update the disable and enable logic to make use of this new flag.
There shouldn't be any functional change (yet) with this patch.
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Reviewed-by: David Francis <David.Francis@amd.com>
Acked-by: Leo Li <sunpeng.li@amd.com>
Signed-off-by: Chaudhary Amit Kumar <Chaudharyamit.Kumar@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 104 +++++++++++++-----
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +
2 files changed, 80 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index ac3ac629d4cb..65c15ae620bd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3525,6 +3525,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
dc_stream_retain(state->stream);
}
+ state->active_planes = cur->active_planes;
+ state->interrupts_enabled = cur->interrupts_enabled;
state->vrr_params = cur->vrr_params;
state->vrr_infopacket = cur->vrr_infopacket;
state->abm_level = cur->abm_level;
@@ -3974,7 +3976,7 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc)
{
}
-static bool does_crtc_have_active_plane(struct drm_crtc_state *new_crtc_state)
+static int count_crtc_active_planes(struct drm_crtc_state *new_crtc_state)
{
struct drm_atomic_state *state = new_crtc_state->state;
struct drm_plane *plane;
@@ -4003,7 +4005,32 @@ static bool does_crtc_have_active_plane(struct drm_crtc_state *new_crtc_state)
num_active += (new_plane_state->fb != NULL);
}
- return num_active > 0;
+ return num_active;
+}
+
+/*
+ * Sets whether interrupts should be enabled on a specific CRTC.
+ * We require that the stream be enabled and that there exist active
+ * DC planes on the stream.
+ */
+static void
+dm_update_crtc_interrupt_state(struct drm_crtc *crtc,
+ struct drm_crtc_state *new_crtc_state)
+{
+ struct dm_crtc_state *dm_new_crtc_state =
+ to_dm_crtc_state(new_crtc_state);
+
+ dm_new_crtc_state->active_planes = 0;
+ dm_new_crtc_state->interrupts_enabled = false;
+
+ if (!dm_new_crtc_state->stream)
+ return;
+
+ dm_new_crtc_state->active_planes =
+ count_crtc_active_planes(new_crtc_state);
+
+ dm_new_crtc_state->interrupts_enabled =
+ dm_new_crtc_state->active_planes > 0;
}
static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
@@ -4014,6 +4041,14 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state);
int ret = -EINVAL;
+ /*
+ * Update interrupt state for the CRTC. This needs to happen whenever
+ * the CRTC has changed or whenever any of its planes have changed.
+ * Atomic check satisfies both of these requirements since the CRTC
+ * is added to the state by DRM during drm_atomic_helper_check_planes.
+ */
+ dm_update_crtc_interrupt_state(crtc, state);
+
if (unlikely(!dm_crtc_state->stream &&
modeset_required(state, NULL, dm_crtc_state->stream))) {
WARN_ON(1);
@@ -4026,7 +4061,7 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc *crtc,
/* We want at least one hardware plane enabled to use the stream. */
if (state->enable && state->active &&
- !does_crtc_have_active_plane(state))
+ dm_crtc_state->active_planes == 0)
return -EINVAL;
if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK)
@@ -5510,28 +5545,43 @@ static int amdgpu_dm_atomic_commit(struct drm_device *dev,
struct amdgpu_device *adev = dev->dev_private;
int i;
- /*
- * We evade vblanks and pflips on crtc that
- * should be changed. We do it here to flush & disable
- * interrupts before drm_swap_state is called in drm_atomic_helper_commit
- * it will update crtc->dm_crtc_state->stream pointer which is used in
- * the ISRs.
+ /*
+ * We evade vblank and pflip interrupts on CRTCs that are undergoing
+ * a modeset, being disabled, or have no active planes.
+ *
+ * It's done in atomic commit rather than commit tail for now since
+ * some of these interrupt handlers access the current CRTC state and
+ * potentially the stream pointer itself.
+ *
+ * Since the atomic state is swapped within atomic commit and not within
+ * commit tail this would leave to new state (that hasn't been committed yet)
+ * being accesssed from within the handlers.
+ *
+ * TODO: Fix this so we can do this in commit tail and not have to block
+ * in atomic check.
*/
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
struct dm_crtc_state *dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
struct dm_crtc_state *dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- if (drm_atomic_crtc_needs_modeset(new_crtc_state)
- && dm_old_crtc_state->stream) {
+ if (dm_old_crtc_state->interrupts_enabled &&
+ (!dm_new_crtc_state->interrupts_enabled ||
+ drm_atomic_crtc_needs_modeset(new_crtc_state))) {
/*
- * CRC capture was enabled but not disabled.
- * Release the vblank reference.
- */
- if (dm_new_crtc_state->crc_enabled) {
- drm_crtc_vblank_put(crtc);
- dm_new_crtc_state->crc_enabled = false;
- }
+ * Drop the extra vblank reference added by CRC
+ * capture if applicable.
+ */
+ if (dm_new_crtc_state->crc_enabled)
+ drm_crtc_vblank_put(crtc);
+
+ /*
+ * Only keep CRC capture enabled if there's
+ * still a stream for the CRTC.
+ */
+ if (!dm_new_crtc_state->stream)
+ dm_new_crtc_state->crc_enabled = false;
+
manage_dm_interrupts(adev, acrtc, false);
}
@@ -5751,13 +5801,14 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
pre_update_freesync_state_on_stream(dm, dm_new_crtc_state);
}
+ /*
+ * Enable interrupts on CRTCs that are newly active, undergone
+ * a modeset, or have active planes again.
+ */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
new_crtc_state, i) {
- /*
- * loop to enable interrupts on newly arrived crtc
- */
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- bool modeset_needed;
+ bool enable;
if (old_crtc_state->active && !new_crtc_state->active)
crtc_disable_count++;
@@ -5769,12 +5820,11 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
amdgpu_dm_handle_vrr_transition(dm_old_crtc_state,
dm_new_crtc_state);
- modeset_needed = modeset_required(
- new_crtc_state,
- dm_new_crtc_state->stream,
- dm_old_crtc_state->stream);
+ enable = dm_new_crtc_state->interrupts_enabled &&
+ (!dm_old_crtc_state->interrupts_enabled ||
+ drm_atomic_crtc_needs_modeset(new_crtc_state));
- if (dm_new_crtc_state->stream == NULL || !modeset_needed)
+ if (!enable)
continue;
manage_dm_interrupts(adev, acrtc, true);
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index e194bf79ca90..715a2d66a0a7 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -268,6 +268,9 @@ struct dm_crtc_state {
struct drm_crtc_state base;
struct dc_stream_state *stream;
+ int active_planes;
+ bool interrupts_enabled;
+
int crc_skip_count;
bool crc_enabled;
--
2.17.1
|
