Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/atomic: track bitmask of planes attached to crtc

Chasing plane->state->crtc of planes that are *not* part of the same
atomic update is racy, making it incredibly awkward (or impossible) to
do something simple like iterate over all planes and figure out which
ones are attached to a crtc.

Solve this by adding a bitmask of currently attached planes in the
crtc-state.

Note that the transitional helpers do not maintain the plane_mask. But
they only support the legacy ioctls, which have sufficient brute-force
locking around plane updates that they can continue to loop over all
planes to see what is attached to a crtc the old way.

Signed-off-by: Rob Clark <robdclark@gmail.com>
[danvet:
- Drop comments about locking in set_crtc_for_plane since they're a
bit misleading - we already should hold lock for the current crtc.
- Also WARN_ON if get_state on the old crtc fails since that should
have been done already.
- Squash in fixup to check get_plane_state return value, reported by
Dan Carpenter and acked by Rob Clark.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

authored by

Rob Clark and committed by
Daniel Vetter
6ddd388a 3009c037

+38 -14
+21 -5
drivers/gpu/drm/drm_atomic.c
··· 344 344 345 345 /** 346 346 * drm_atomic_set_crtc_for_plane - set crtc for plane 347 - * @plane_state: atomic state object for the plane 347 + * @state: the incoming atomic state 348 + * @plane: the plane whose incoming state to update 348 349 * @crtc: crtc to use for the plane 349 350 * 350 351 * Changing the assigned crtc for a plane requires us to grab the lock and state ··· 358 357 * sequence must be restarted. All other errors are fatal. 359 358 */ 360 359 int 361 - drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, 362 - struct drm_crtc *crtc) 360 + drm_atomic_set_crtc_for_plane(struct drm_atomic_state *state, 361 + struct drm_plane *plane, struct drm_crtc *crtc) 363 362 { 363 + struct drm_plane_state *plane_state = 364 + drm_atomic_get_plane_state(state, plane); 364 365 struct drm_crtc_state *crtc_state; 366 + 367 + if (WARN_ON(IS_ERR(plane_state))) 368 + return PTR_ERR(plane_state); 369 + 370 + if (plane_state->crtc) { 371 + crtc_state = drm_atomic_get_crtc_state(plane_state->state, 372 + plane_state->crtc); 373 + if (WARN_ON(IS_ERR(crtc_state))) 374 + return PTR_ERR(crtc_state); 375 + 376 + crtc_state->plane_mask &= ~(1 << drm_plane_index(plane)); 377 + } 378 + 379 + plane_state->crtc = crtc; 365 380 366 381 if (crtc) { 367 382 crtc_state = drm_atomic_get_crtc_state(plane_state->state, 368 383 crtc); 369 384 if (IS_ERR(crtc_state)) 370 385 return PTR_ERR(crtc_state); 386 + crtc_state->plane_mask |= (1 << drm_plane_index(plane)); 371 387 } 372 - 373 - plane_state->crtc = crtc; 374 388 375 389 if (crtc) 376 390 DRM_DEBUG_KMS("Link plane state %p to [CRTC:%d]\n",
+4 -4
drivers/gpu/drm/drm_atomic_helper.c
··· 1222 1222 goto fail; 1223 1223 } 1224 1224 1225 - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); 1225 + ret = drm_atomic_set_crtc_for_plane(state, plane, crtc); 1226 1226 if (ret != 0) 1227 1227 goto fail; 1228 1228 drm_atomic_set_fb_for_plane(plane_state, fb); ··· 1301 1301 goto fail; 1302 1302 } 1303 1303 1304 - ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); 1304 + ret = drm_atomic_set_crtc_for_plane(state, plane, NULL); 1305 1305 if (ret != 0) 1306 1306 goto fail; 1307 1307 drm_atomic_set_fb_for_plane(plane_state, NULL); ··· 1472 1472 goto fail; 1473 1473 } 1474 1474 1475 - ret = drm_atomic_set_crtc_for_plane(primary_state, crtc); 1475 + ret = drm_atomic_set_crtc_for_plane(state, crtc->primary, crtc); 1476 1476 if (ret != 0) 1477 1477 goto fail; 1478 1478 drm_atomic_set_fb_for_plane(primary_state, set->fb); ··· 1744 1744 goto fail; 1745 1745 } 1746 1746 1747 - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); 1747 + ret = drm_atomic_set_crtc_for_plane(state, plane, crtc); 1748 1748 if (ret != 0) 1749 1749 goto fail; 1750 1750 drm_atomic_set_fb_for_plane(plane_state, fb);
+2 -2
include/drm/drm_atomic.h
··· 46 46 struct drm_connector *connector); 47 47 48 48 int __must_check 49 - drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state, 50 - struct drm_crtc *crtc); 49 + drm_atomic_set_crtc_for_plane(struct drm_atomic_state *state, 50 + struct drm_plane *plane, struct drm_crtc *crtc); 51 51 void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state, 52 52 struct drm_framebuffer *fb); 53 53 int __must_check
+11 -3
include/drm/drm_crtc.h
··· 231 231 * struct drm_crtc_state - mutable CRTC state 232 232 * @enable: whether the CRTC should be enabled, gates all other state 233 233 * @mode_changed: for use by helpers and drivers when computing state updates 234 + * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes 234 235 * @last_vblank_count: for helpers and drivers to capture the vblank of the 235 236 * update to ensure framebuffer cleanup isn't done too early 236 237 * @planes_changed: for use by helpers and drivers when computing state updates ··· 247 246 /* computed state bits used by helpers and drivers */ 248 247 bool planes_changed : 1; 249 248 bool mode_changed : 1; 249 + 250 + /* attached planes bitmask: 251 + * WARNING: transitional helpers do not maintain plane_mask so 252 + * drivers not converted over to atomic helpers should not rely 253 + * on plane_mask being accurate! 254 + */ 255 + u32 plane_mask; 250 256 251 257 /* last_vblank_count: for vblank waits before cleanup */ 252 258 u32 last_vblank_count; ··· 446 438 * @state: backpointer to global drm_atomic_state 447 439 */ 448 440 struct drm_connector_state { 449 - struct drm_crtc *crtc; 441 + struct drm_crtc *crtc; /* do not write directly, use drm_atomic_set_crtc_for_connector() */ 450 442 451 443 struct drm_encoder *best_encoder; 452 444 ··· 681 673 * @state: backpointer to global drm_atomic_state 682 674 */ 683 675 struct drm_plane_state { 684 - struct drm_crtc *crtc; 685 - struct drm_framebuffer *fb; 676 + struct drm_crtc *crtc; /* do not write directly, use drm_atomic_set_crtc_for_plane() */ 677 + struct drm_framebuffer *fb; /* do not write directly, use drm_atomic_set_fb_for_plane() */ 686 678 struct fence *fence; 687 679 688 680 /* Signed dest location allows it to be partially off screen */