While creating display and event threads per crtc, validate them before setting their priorities.
changes in v2: - use dev_warn (Abhinav Kumar)
Change-Id: I1dda805286df981c0f0e2b26507d089d3a21ff6c Signed-off-by: Jeykumar Sankaran jsanka@codeaurora.org --- drivers/gpu/drm/msm/msm_drv.c | 49 ++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 4904d0d..9df4047 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -553,17 +553,18 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) kthread_run(kthread_worker_fn, &priv->disp_thread[i].worker, "crtc_commit:%d", priv->disp_thread[i].crtc_id); - ret = sched_setscheduler(priv->disp_thread[i].thread, - SCHED_FIFO, ¶m); - if (ret) - pr_warn("display thread priority update failed: %d\n", - ret); - if (IS_ERR(priv->disp_thread[i].thread)) { dev_err(dev, "failed to create crtc_commit kthread\n"); priv->disp_thread[i].thread = NULL; + goto err_msm_uninit; }
+ ret = sched_setscheduler(priv->disp_thread[i].thread, + SCHED_FIFO, ¶m); + if (ret) + dev_warn("display thread priority update failed: %d\n", + ret); + /* initialize event thread */ priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id; kthread_init_worker(&priv->event_thread[i].worker); @@ -572,6 +573,12 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) kthread_run(kthread_worker_fn, &priv->event_thread[i].worker, "crtc_event:%d", priv->event_thread[i].crtc_id); + if (IS_ERR(priv->event_thread[i].thread)) { + dev_err(dev, "failed to create crtc_event kthread\n"); + priv->event_thread[i].thread = NULL; + goto err_msm_uninit; + } + /** * event thread should also run at same priority as disp_thread * because it is handling frame_done events. A lower priority @@ -580,34 +587,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv) * failure at crtc commit level. */ ret = sched_setscheduler(priv->event_thread[i].thread, - SCHED_FIFO, ¶m); + SCHED_FIFO, ¶m); if (ret) - pr_warn("display event thread priority update failed: %d\n", - ret); - - if (IS_ERR(priv->event_thread[i].thread)) { - dev_err(dev, "failed to create crtc_event kthread\n"); - priv->event_thread[i].thread = NULL; - } - - if ((!priv->disp_thread[i].thread) || - !priv->event_thread[i].thread) { - /* clean up previously created threads if any */ - for ( ; i >= 0; i--) { - if (priv->disp_thread[i].thread) { - kthread_stop( - priv->disp_thread[i].thread); - priv->disp_thread[i].thread = NULL; - } - - if (priv->event_thread[i].thread) { - kthread_stop( - priv->event_thread[i].thread); - priv->event_thread[i].thread = NULL; - } - } - goto err_msm_uninit; - } + dev_warn("display event thread priority update failed:%d\n", + ret); }
ret = drm_vblank_init(ddev, priv->num_crtcs);
Hi Jeykumar,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robclark/msm-next] [also build test ERROR on v4.19-rc7 next-20181009] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jeykumar-Sankaran/drm-msm-validate-... base: git://people.freedesktop.org/~robclark/linux msm-next config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
In file included from include/linux/hdmi.h:28:0, from include/drm/drm_modes.h:30, from include/drm/drm_bridge.h:29, from include/drm/drm_of.h:7, from drivers/gpu//drm/msm/msm_drv.c:21: drivers/gpu//drm/msm/msm_drv.c: In function 'msm_drm_init':
drivers/gpu//drm/msm/msm_drv.c:565:13: error: passing argument 1 of '_dev_warn' from incompatible pointer type [-Werror=incompatible-pointer-types]
dev_warn("display thread priority update failed: %d\n", ^ include/linux/device.h:1420:12: note: in definition of macro 'dev_warn' _dev_warn(dev, dev_fmt(fmt), ##__VA_ARGS__) ^~~ include/linux/device.h:1358:6: note: expected 'const struct device *' but argument is of type 'char *' void _dev_warn(const struct device *dev, const char *fmt, ...); ^~~~~~~~~
drivers/gpu//drm/msm/msm_drv.c:566:5: warning: passing argument 2 of '_dev_warn' makes pointer from integer without a cast [-Wint-conversion]
ret); ^ include/linux/device.h:1335:22: note: in definition of macro 'dev_fmt' #define dev_fmt(fmt) fmt ^~~
drivers/gpu//drm/msm/msm_drv.c:565:4: note: in expansion of macro 'dev_warn'
dev_warn("display thread priority update failed: %d\n", ^~~~~~~~ include/linux/device.h:1358:6: note: expected 'const char *' but argument is of type 'int' void _dev_warn(const struct device *dev, const char *fmt, ...); ^~~~~~~~~ drivers/gpu//drm/msm/msm_drv.c:592:13: error: passing argument 1 of '_dev_warn' from incompatible pointer type [-Werror=incompatible-pointer-types] dev_warn("display event thread priority update failed:%d\n", ^ include/linux/device.h:1420:12: note: in definition of macro 'dev_warn' _dev_warn(dev, dev_fmt(fmt), ##__VA_ARGS__) ^~~ include/linux/device.h:1358:6: note: expected 'const struct device *' but argument is of type 'char *' void _dev_warn(const struct device *dev, const char *fmt, ...); ^~~~~~~~~ drivers/gpu//drm/msm/msm_drv.c:593:5: warning: passing argument 2 of '_dev_warn' makes pointer from integer without a cast [-Wint-conversion] ret); ^ include/linux/device.h:1335:22: note: in definition of macro 'dev_fmt' #define dev_fmt(fmt) fmt ^~~ drivers/gpu//drm/msm/msm_drv.c:592:4: note: in expansion of macro 'dev_warn' dev_warn("display event thread priority update failed:%d\n", ^~~~~~~~ include/linux/device.h:1358:6: note: expected 'const char *' but argument is of type 'int' void _dev_warn(const struct device *dev, const char *fmt, ...); ^~~~~~~~~ cc1: some warnings being treated as errors
vim +/dev_warn +565 drivers/gpu//drm/msm/msm_drv.c
433 434 static int msm_drm_init(struct device *dev, struct drm_driver *drv) 435 { 436 struct platform_device *pdev = to_platform_device(dev); 437 struct drm_device *ddev; 438 struct msm_drm_private *priv; 439 struct msm_kms *kms; 440 struct msm_mdss *mdss; 441 int ret, i; 442 struct sched_param param; 443 444 ddev = drm_dev_alloc(drv, dev); 445 if (IS_ERR(ddev)) { 446 dev_err(dev, "failed to allocate drm_device\n"); 447 return PTR_ERR(ddev); 448 } 449 450 platform_set_drvdata(pdev, ddev); 451 452 priv = kzalloc(sizeof(*priv), GFP_KERNEL); 453 if (!priv) { 454 ret = -ENOMEM; 455 goto err_put_drm_dev; 456 } 457 458 ddev->dev_private = priv; 459 priv->dev = ddev; 460 461 switch (get_mdp_ver(pdev)) { 462 case KMS_MDP5: 463 ret = mdp5_mdss_init(ddev); 464 break; 465 case KMS_DPU: 466 ret = dpu_mdss_init(ddev); 467 break; 468 default: 469 ret = 0; 470 break; 471 } 472 if (ret) 473 goto err_free_priv; 474 475 mdss = priv->mdss; 476 477 priv->wq = alloc_ordered_workqueue("msm", 0); 478 479 INIT_LIST_HEAD(&priv->inactive_list); 480 INIT_LIST_HEAD(&priv->vblank_ctrl.event_list); 481 kthread_init_work(&priv->vblank_ctrl.work, vblank_ctrl_worker); 482 spin_lock_init(&priv->vblank_ctrl.lock); 483 484 drm_mode_config_init(ddev); 485 486 /* Bind all our sub-components: */ 487 ret = component_bind_all(dev, ddev); 488 if (ret) 489 goto err_destroy_mdss; 490 491 ret = msm_init_vram(ddev); 492 if (ret) 493 goto err_msm_uninit; 494 495 msm_gem_shrinker_init(ddev); 496 497 switch (get_mdp_ver(pdev)) { 498 case KMS_MDP4: 499 kms = mdp4_kms_init(ddev); 500 priv->kms = kms; 501 break; 502 case KMS_MDP5: 503 kms = mdp5_kms_init(ddev); 504 break; 505 case KMS_DPU: 506 kms = dpu_kms_init(ddev); 507 priv->kms = kms; 508 break; 509 default: 510 kms = ERR_PTR(-ENODEV); 511 break; 512 } 513 514 if (IS_ERR(kms)) { 515 /* 516 * NOTE: once we have GPU support, having no kms should not 517 * be considered fatal.. ideally we would still support gpu 518 * and (for example) use dmabuf/prime to share buffers with 519 * imx drm driver on iMX5 520 */ 521 dev_err(dev, "failed to load kms\n"); 522 ret = PTR_ERR(kms); 523 goto err_msm_uninit; 524 } 525 526 /* Enable normalization of plane zpos */ 527 ddev->mode_config.normalize_zpos = true; 528 529 if (kms) { 530 ret = kms->funcs->hw_init(kms); 531 if (ret) { 532 dev_err(dev, "kms hw init failed: %d\n", ret); 533 goto err_msm_uninit; 534 } 535 } 536 537 ddev->mode_config.funcs = &mode_config_funcs; 538 ddev->mode_config.helper_private = &mode_config_helper_funcs; 539 540 /** 541 * this priority was found during empiric testing to have appropriate 542 * realtime scheduling to process display updates and interact with 543 * other real time and normal priority task 544 */ 545 param.sched_priority = 16; 546 for (i = 0; i < priv->num_crtcs; i++) { 547 548 /* initialize display thread */ 549 priv->disp_thread[i].crtc_id = priv->crtcs[i]->base.id; 550 kthread_init_worker(&priv->disp_thread[i].worker); 551 priv->disp_thread[i].dev = ddev; 552 priv->disp_thread[i].thread = 553 kthread_run(kthread_worker_fn, 554 &priv->disp_thread[i].worker, 555 "crtc_commit:%d", priv->disp_thread[i].crtc_id); 556 if (IS_ERR(priv->disp_thread[i].thread)) { 557 dev_err(dev, "failed to create crtc_commit kthread\n"); 558 priv->disp_thread[i].thread = NULL; 559 goto err_msm_uninit; 560 } 561 562 ret = sched_setscheduler(priv->disp_thread[i].thread, 563 SCHED_FIFO, ¶m); 564 if (ret)
565 dev_warn("display thread priority update failed: %d\n", 566 ret);
567 568 /* initialize event thread */ 569 priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id; 570 kthread_init_worker(&priv->event_thread[i].worker); 571 priv->event_thread[i].dev = ddev; 572 priv->event_thread[i].thread = 573 kthread_run(kthread_worker_fn, 574 &priv->event_thread[i].worker, 575 "crtc_event:%d", priv->event_thread[i].crtc_id); 576 if (IS_ERR(priv->event_thread[i].thread)) { 577 dev_err(dev, "failed to create crtc_event kthread\n"); 578 priv->event_thread[i].thread = NULL; 579 goto err_msm_uninit; 580 } 581 582 /** 583 * event thread should also run at same priority as disp_thread 584 * because it is handling frame_done events. A lower priority 585 * event thread and higher priority disp_thread can causes 586 * frame_pending counters beyond 2. This can lead to commit 587 * failure at crtc commit level. 588 */ 589 ret = sched_setscheduler(priv->event_thread[i].thread, 590 SCHED_FIFO, ¶m); 591 if (ret) 592 dev_warn("display event thread priority update failed:%d\n", 593 ret); 594 } 595 596 ret = drm_vblank_init(ddev, priv->num_crtcs); 597 if (ret < 0) { 598 dev_err(dev, "failed to initialize vblank\n"); 599 goto err_msm_uninit; 600 } 601 602 if (kms) { 603 pm_runtime_get_sync(dev); 604 ret = drm_irq_install(ddev, kms->irq); 605 pm_runtime_put_sync(dev); 606 if (ret < 0) { 607 dev_err(dev, "failed to install IRQ handler\n"); 608 goto err_msm_uninit; 609 } 610 } 611 612 ret = drm_dev_register(ddev, 0); 613 if (ret) 614 goto err_msm_uninit; 615 616 drm_mode_config_reset(ddev); 617
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
dri-devel@lists.freedesktop.org
-
Jeykumar Sankaran -
kbuild test robot