| Message ID | 1539041538-18152-1-git-send-email-jsanka@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/msm: validate display and event threads | expand |
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-display-and-event-threads/20181009-203659
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
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);
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(-)