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

drm: Remove transitional helpers

With armada the last bigger driver that realistically needed these to
convert from legacy kms to atomic is converted. These helpers have
been broken more often than not the past 2 years, and as this little
patch series shows, tricked a bunch of people into using the wrong
helpers for their functions.

Aside: I think a lot more drivers should be using the device-level
drm_atomic_helper_shutdown/suspend/resume helpers and related
functions. In almost all the cases they get things exactly right.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181004202446.22905-16-daniel.vetter@ffwll.ch

-332
-115
drivers/gpu/drm/drm_crtc_helper.c
··· 984 984 drm_modeset_unlock_all(dev); 985 985 } 986 986 EXPORT_SYMBOL(drm_helper_resume_force_mode); 987 - 988 - /** 989 - * drm_helper_crtc_mode_set - mode_set implementation for atomic plane helpers 990 - * @crtc: DRM CRTC 991 - * @mode: DRM display mode which userspace requested 992 - * @adjusted_mode: DRM display mode adjusted by ->mode_fixup callbacks 993 - * @x: x offset of the CRTC scanout area on the underlying framebuffer 994 - * @y: y offset of the CRTC scanout area on the underlying framebuffer 995 - * @old_fb: previous framebuffer 996 - * 997 - * This function implements a callback useable as the ->mode_set callback 998 - * required by the CRTC helpers. Besides the atomic plane helper functions for 999 - * the primary plane the driver must also provide the ->mode_set_nofb callback 1000 - * to set up the CRTC. 1001 - * 1002 - * This is a transitional helper useful for converting drivers to the atomic 1003 - * interfaces. 1004 - */ 1005 - int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, 1006 - struct drm_display_mode *adjusted_mode, int x, int y, 1007 - struct drm_framebuffer *old_fb) 1008 - { 1009 - struct drm_crtc_state *crtc_state; 1010 - const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; 1011 - int ret; 1012 - 1013 - if (crtc->funcs->atomic_duplicate_state) 1014 - crtc_state = crtc->funcs->atomic_duplicate_state(crtc); 1015 - else { 1016 - if (!crtc->state) 1017 - drm_atomic_helper_crtc_reset(crtc); 1018 - 1019 - crtc_state = drm_atomic_helper_crtc_duplicate_state(crtc); 1020 - } 1021 - 1022 - if (!crtc_state) 1023 - return -ENOMEM; 1024 - 1025 - crtc_state->planes_changed = true; 1026 - crtc_state->mode_changed = true; 1027 - ret = drm_atomic_set_mode_for_crtc(crtc_state, mode); 1028 - if (ret) 1029 - goto out; 1030 - drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode); 1031 - 1032 - if (crtc_funcs->atomic_check) { 1033 - ret = crtc_funcs->atomic_check(crtc, crtc_state); 1034 - if (ret) 1035 - goto out; 1036 - } 1037 - 1038 - swap(crtc->state, crtc_state); 1039 - 1040 - crtc_funcs->mode_set_nofb(crtc); 1041 - 1042 - ret = drm_helper_crtc_mode_set_base(crtc, x, y, old_fb); 1043 - 1044 - out: 1045 - if (crtc_state) { 1046 - if (crtc->funcs->atomic_destroy_state) 1047 - crtc->funcs->atomic_destroy_state(crtc, crtc_state); 1048 - else 1049 - drm_atomic_helper_crtc_destroy_state(crtc, crtc_state); 1050 - } 1051 - 1052 - return ret; 1053 - } 1054 - EXPORT_SYMBOL(drm_helper_crtc_mode_set); 1055 - 1056 - /** 1057 - * drm_helper_crtc_mode_set_base - mode_set_base implementation for atomic plane helpers 1058 - * @crtc: DRM CRTC 1059 - * @x: x offset of the CRTC scanout area on the underlying framebuffer 1060 - * @y: y offset of the CRTC scanout area on the underlying framebuffer 1061 - * @old_fb: previous framebuffer 1062 - * 1063 - * This function implements a callback useable as the ->mode_set_base used 1064 - * required by the CRTC helpers. The driver must provide the atomic plane helper 1065 - * functions for the primary plane. 1066 - * 1067 - * This is a transitional helper useful for converting drivers to the atomic 1068 - * interfaces. 1069 - */ 1070 - int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, 1071 - struct drm_framebuffer *old_fb) 1072 - { 1073 - struct drm_plane_state *plane_state; 1074 - struct drm_plane *plane = crtc->primary; 1075 - 1076 - if (plane->funcs->atomic_duplicate_state) 1077 - plane_state = plane->funcs->atomic_duplicate_state(plane); 1078 - else { 1079 - if (!plane->state) 1080 - drm_atomic_helper_plane_reset(plane); 1081 - 1082 - plane_state = drm_atomic_helper_plane_duplicate_state(plane); 1083 - } 1084 - if (!plane_state) 1085 - return -ENOMEM; 1086 - plane_state->plane = plane; 1087 - 1088 - plane_state->crtc = crtc; 1089 - drm_atomic_set_fb_for_plane(plane_state, crtc->primary->fb); 1090 - plane_state->crtc_x = 0; 1091 - plane_state->crtc_y = 0; 1092 - plane_state->crtc_h = crtc->mode.vdisplay; 1093 - plane_state->crtc_w = crtc->mode.hdisplay; 1094 - plane_state->src_x = x << 16; 1095 - plane_state->src_y = y << 16; 1096 - plane_state->src_h = crtc->mode.vdisplay << 16; 1097 - plane_state->src_w = crtc->mode.hdisplay << 16; 1098 - 1099 - return drm_plane_helper_commit(plane, plane_state, old_fb); 1100 - } 1101 - EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
-197
drivers/gpu/drm/drm_plane_helper.c
··· 336 336 .destroy = drm_primary_helper_destroy, 337 337 }; 338 338 EXPORT_SYMBOL(drm_primary_helper_funcs); 339 - 340 - int drm_plane_helper_commit(struct drm_plane *plane, 341 - struct drm_plane_state *plane_state, 342 - struct drm_framebuffer *old_fb) 343 - { 344 - const struct drm_plane_helper_funcs *plane_funcs; 345 - struct drm_crtc *crtc[2]; 346 - const struct drm_crtc_helper_funcs *crtc_funcs[2]; 347 - int i, ret = 0; 348 - 349 - plane_funcs = plane->helper_private; 350 - 351 - /* Since this is a transitional helper we can't assume that plane->state 352 - * is always valid. Hence we need to use plane->crtc instead of 353 - * plane->state->crtc as the old crtc. */ 354 - crtc[0] = plane->crtc; 355 - crtc[1] = crtc[0] != plane_state->crtc ? plane_state->crtc : NULL; 356 - 357 - for (i = 0; i < 2; i++) 358 - crtc_funcs[i] = crtc[i] ? crtc[i]->helper_private : NULL; 359 - 360 - if (plane_funcs->atomic_check) { 361 - ret = plane_funcs->atomic_check(plane, plane_state); 362 - if (ret) 363 - goto out; 364 - } 365 - 366 - if (plane_funcs->prepare_fb && plane_state->fb != old_fb) { 367 - ret = plane_funcs->prepare_fb(plane, 368 - plane_state); 369 - if (ret) 370 - goto out; 371 - } 372 - 373 - /* Point of no return, commit sw state. */ 374 - swap(plane->state, plane_state); 375 - 376 - for (i = 0; i < 2; i++) { 377 - if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin) 378 - crtc_funcs[i]->atomic_begin(crtc[i], crtc[i]->state); 379 - } 380 - 381 - /* 382 - * Drivers may optionally implement the ->atomic_disable callback, so 383 - * special-case that here. 384 - */ 385 - if (drm_atomic_plane_disabling(plane_state, plane->state) && 386 - plane_funcs->atomic_disable) 387 - plane_funcs->atomic_disable(plane, plane_state); 388 - else 389 - plane_funcs->atomic_update(plane, plane_state); 390 - 391 - for (i = 0; i < 2; i++) { 392 - if (crtc_funcs[i] && crtc_funcs[i]->atomic_flush) 393 - crtc_funcs[i]->atomic_flush(crtc[i], crtc[i]->state); 394 - } 395 - 396 - /* 397 - * If we only moved the plane and didn't change fb's, there's no need to 398 - * wait for vblank. 399 - */ 400 - if (plane->state->fb == old_fb) 401 - goto out; 402 - 403 - for (i = 0; i < 2; i++) { 404 - if (!crtc[i]) 405 - continue; 406 - 407 - if (crtc[i]->cursor == plane) 408 - continue; 409 - 410 - /* There's no other way to figure out whether the crtc is running. */ 411 - ret = drm_crtc_vblank_get(crtc[i]); 412 - if (ret == 0) { 413 - drm_crtc_wait_one_vblank(crtc[i]); 414 - drm_crtc_vblank_put(crtc[i]); 415 - } 416 - 417 - ret = 0; 418 - } 419 - 420 - if (plane_funcs->cleanup_fb) 421 - plane_funcs->cleanup_fb(plane, plane_state); 422 - out: 423 - if (plane->funcs->atomic_destroy_state) 424 - plane->funcs->atomic_destroy_state(plane, plane_state); 425 - else 426 - drm_atomic_helper_plane_destroy_state(plane, plane_state); 427 - 428 - return ret; 429 - } 430 - 431 - /** 432 - * drm_plane_helper_update() - Transitional helper for plane update 433 - * @plane: plane object to update 434 - * @crtc: owning CRTC of owning plane 435 - * @fb: framebuffer to flip onto plane 436 - * @crtc_x: x offset of primary plane on crtc 437 - * @crtc_y: y offset of primary plane on crtc 438 - * @crtc_w: width of primary plane rectangle on crtc 439 - * @crtc_h: height of primary plane rectangle on crtc 440 - * @src_x: x offset of @fb for panning 441 - * @src_y: y offset of @fb for panning 442 - * @src_w: width of source rectangle in @fb 443 - * @src_h: height of source rectangle in @fb 444 - * @ctx: lock acquire context, not used here 445 - * 446 - * Provides a default plane update handler using the atomic plane update 447 - * functions. It is fully left to the driver to check plane constraints and 448 - * handle corner-cases like a fully occluded or otherwise invisible plane. 449 - * 450 - * This is useful for piecewise transitioning of a driver to the atomic helpers. 451 - * 452 - * RETURNS: 453 - * Zero on success, error code on failure 454 - */ 455 - int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, 456 - struct drm_framebuffer *fb, 457 - int crtc_x, int crtc_y, 458 - unsigned int crtc_w, unsigned int crtc_h, 459 - uint32_t src_x, uint32_t src_y, 460 - uint32_t src_w, uint32_t src_h, 461 - struct drm_modeset_acquire_ctx *ctx) 462 - { 463 - struct drm_plane_state *plane_state; 464 - 465 - if (plane->funcs->atomic_duplicate_state) 466 - plane_state = plane->funcs->atomic_duplicate_state(plane); 467 - else { 468 - if (!plane->state) 469 - drm_atomic_helper_plane_reset(plane); 470 - 471 - plane_state = drm_atomic_helper_plane_duplicate_state(plane); 472 - } 473 - if (!plane_state) 474 - return -ENOMEM; 475 - plane_state->plane = plane; 476 - 477 - plane_state->crtc = crtc; 478 - drm_atomic_set_fb_for_plane(plane_state, fb); 479 - plane_state->crtc_x = crtc_x; 480 - plane_state->crtc_y = crtc_y; 481 - plane_state->crtc_h = crtc_h; 482 - plane_state->crtc_w = crtc_w; 483 - plane_state->src_x = src_x; 484 - plane_state->src_y = src_y; 485 - plane_state->src_h = src_h; 486 - plane_state->src_w = src_w; 487 - 488 - return drm_plane_helper_commit(plane, plane_state, plane->fb); 489 - } 490 - EXPORT_SYMBOL(drm_plane_helper_update); 491 - 492 - /** 493 - * drm_plane_helper_disable() - Transitional helper for plane disable 494 - * @plane: plane to disable 495 - * @ctx: lock acquire context, not used here 496 - * 497 - * Provides a default plane disable handler using the atomic plane update 498 - * functions. It is fully left to the driver to check plane constraints and 499 - * handle corner-cases like a fully occluded or otherwise invisible plane. 500 - * 501 - * This is useful for piecewise transitioning of a driver to the atomic helpers. 502 - * 503 - * RETURNS: 504 - * Zero on success, error code on failure 505 - */ 506 - int drm_plane_helper_disable(struct drm_plane *plane, 507 - struct drm_modeset_acquire_ctx *ctx) 508 - { 509 - struct drm_plane_state *plane_state; 510 - struct drm_framebuffer *old_fb; 511 - 512 - /* crtc helpers love to call disable functions for already disabled hw 513 - * functions. So cope with that. */ 514 - if (!plane->crtc) 515 - return 0; 516 - 517 - if (plane->funcs->atomic_duplicate_state) 518 - plane_state = plane->funcs->atomic_duplicate_state(plane); 519 - else { 520 - if (!plane->state) 521 - drm_atomic_helper_plane_reset(plane); 522 - 523 - plane_state = drm_atomic_helper_plane_duplicate_state(plane); 524 - } 525 - if (!plane_state) 526 - return -ENOMEM; 527 - plane_state->plane = plane; 528 - 529 - plane_state->crtc = NULL; 530 - old_fb = plane_state->fb; 531 - drm_atomic_set_fb_for_plane(plane_state, NULL); 532 - 533 - return drm_plane_helper_commit(plane, plane_state, old_fb); 534 - } 535 - EXPORT_SYMBOL(drm_plane_helper_disable);
-6
include/drm/drm_crtc_helper.h
··· 57 57 58 58 void drm_helper_resume_force_mode(struct drm_device *dev); 59 59 60 - int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, 61 - struct drm_display_mode *adjusted_mode, int x, int y, 62 - struct drm_framebuffer *old_fb); 63 - int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y, 64 - struct drm_framebuffer *old_fb); 65 - 66 60 /* drm_probe_helper.c */ 67 61 int drm_helper_probe_single_connector_modes(struct drm_connector 68 62 *connector, uint32_t maxX,
-14
include/drm/drm_plane_helper.h
··· 62 62 void drm_primary_helper_destroy(struct drm_plane *plane); 63 63 extern const struct drm_plane_funcs drm_primary_helper_funcs; 64 64 65 - int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc, 66 - struct drm_framebuffer *fb, 67 - int crtc_x, int crtc_y, 68 - unsigned int crtc_w, unsigned int crtc_h, 69 - uint32_t src_x, uint32_t src_y, 70 - uint32_t src_w, uint32_t src_h, 71 - struct drm_modeset_acquire_ctx *ctx); 72 - int drm_plane_helper_disable(struct drm_plane *plane, 73 - struct drm_modeset_acquire_ctx *ctx); 74 - 75 - /* For use by drm_crtc_helper.c */ 76 - int drm_plane_helper_commit(struct drm_plane *plane, 77 - struct drm_plane_state *plane_state, 78 - struct drm_framebuffer *old_fb); 79 65 #endif