Skip to content

Commit cb1824b

Browse files
committed
drm/i915: Fix crtc nv12 etc. plane bitmasks for DPMS off
We only consider crtc_state->enable when initially calculating plane visibility. Later on we try to override the plane's state to invisible if the crtc is in DPMS off state (crtc_state->active==false). Unfortunately the code doing that only updates the plane_state.visible flag and the crtc_state.active_planes bimask, but forgets to update some of the other plane bitmasks stored in the crtc_state. Namely crtc_state.nv12_planes is left set up based on the original visibility check which makes icl_check_nv12_planes() pick a slave plane for the flagged plane in the bitmask. Later on we hit the watermark code which sees a plane with a slave assigned and it then makes the logical assumption that the master plane must itself be visible. Since the master's plane_state.visible flag was already cleared we get a WARN. Fix the problem by clearing all the plane bitmasks for DPMS off. This is more or less the wrong approach and instead we should calculate all the plane related state purely based crtc_state->enable (to guarantee that the subsequent DPMS on can't fail). However in the past we definitely had some roadblocks to making that happen. Not sure how many are left these days, but let's stick to the current approach since it's a much simpler fix to the immediate problem (the WARN). v2: Keep the visible=false, it's important (Rodrigo) Cc: Rodrigo Vivi <[email protected]> Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Rodrigo Vivi <[email protected]>
1 parent 03c761b commit cb1824b

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

drivers/gpu/drm/i915/display/intel_atomic_plane.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,20 @@ void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
264264
plane_state->hw.color_range = from_plane_state->uapi.color_range;
265265
}
266266

267+
void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
268+
struct intel_plane_state *plane_state)
269+
{
270+
struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
271+
272+
crtc_state->active_planes &= ~BIT(plane->id);
273+
crtc_state->nv12_planes &= ~BIT(plane->id);
274+
crtc_state->c8_planes &= ~BIT(plane->id);
275+
crtc_state->data_rate[plane->id] = 0;
276+
crtc_state->min_cdclk[plane->id] = 0;
277+
278+
plane_state->uapi.visible = false;
279+
}
280+
267281
int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state,
268282
struct intel_crtc_state *new_crtc_state,
269283
const struct intel_plane_state *old_plane_state,
@@ -273,12 +287,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
273287
const struct drm_framebuffer *fb = new_plane_state->hw.fb;
274288
int ret;
275289

276-
new_crtc_state->active_planes &= ~BIT(plane->id);
277-
new_crtc_state->nv12_planes &= ~BIT(plane->id);
278-
new_crtc_state->c8_planes &= ~BIT(plane->id);
279-
new_crtc_state->data_rate[plane->id] = 0;
280-
new_crtc_state->min_cdclk[plane->id] = 0;
281-
new_plane_state->uapi.visible = false;
290+
intel_plane_set_invisible(new_crtc_state, new_plane_state);
282291

283292
if (!new_plane_state->hw.crtc && !old_plane_state->hw.crtc)
284293
return 0;

drivers/gpu/drm/i915/display/intel_atomic_plane.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
5252
int intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
5353
struct intel_plane *plane,
5454
bool *need_cdclk_calc);
55+
void intel_plane_set_invisible(struct intel_crtc_state *crtc_state,
56+
struct intel_plane_state *plane_state);
5557

5658
#endif /* __INTEL_ATOMIC_PLANE_H__ */

drivers/gpu/drm/i915/display/intel_display.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12377,10 +12377,8 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
1237712377
* only combine the results from all planes in the current place?
1237812378
*/
1237912379
if (!is_crtc_enabled) {
12380-
plane_state->uapi.visible = visible = false;
12381-
crtc_state->active_planes &= ~BIT(plane->id);
12382-
crtc_state->data_rate[plane->id] = 0;
12383-
crtc_state->min_cdclk[plane->id] = 0;
12380+
intel_plane_set_invisible(crtc_state, plane_state);
12381+
visible = false;
1238412382
}
1238512383

1238612384
if (!was_visible && !visible)

0 commit comments

Comments
 (0)