On Mon, Oct 08, 2018 at 04:55:45PM -0700, Jeykumar Sankaran wrote:
> While creating display and event threads per crtc, validate
> them before setting their priorities.
>
> changes in v2:
> - use dev_warn (Abhinav Kumar)
> changes in v3:
> - fix compilation error
>
> Change-Id: I1dda805286df981c0f0e2b26507d089d3a21ff6c
No Change-Id's please!
FWIW, checkpatch.pl would have caught this and the nits I found below. It might
be worth running patches through it before sending.
> Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>
> ---
> 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..ab1b0a9 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(dev, "display thread priority update failed: %d\n",
Although this is wrapped, the line still exceeds 80 chars. Perhaps:
dev_warn(dev, "disp_thread set priority failed %d\n",
ret);
> + 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(dev, "display event thread priority update failed:%d\n",
Same problem here,
dev_warn(dev, "event thread set priority failed %d\n",
> + ret);
> }
>
> ret = drm_vblank_init(ddev, priv->num_crtcs);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
