Re: [Freedreno] [PATCH v3] drm/msm: validate display and event threads
On 2018-10-09 07:24, Sean Paul wrote:
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
---
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,
>disp_thread[i].worker,
"crtc_commit:%d",
priv->disp_thread[i].crtc_id);
- ret = sched_setscheduler(priv->disp_thread[i].thread,
- SCHED_FIFO,
);
- 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, );
+ 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);
I did run the checkpath.pl on this patch. Check patch usually will not
complain
if the >80 exceeding line is the logging string.
https://01.org/linuxgraphics/gfx-docs/drm/process/coding-style.html#breaking-long-lines-and-strings
https://gitlab.freedesktop.org/seanpaul/dpu-staging/blob/for-next/scripts/checkpatch.pl#L3058
Anyway, rewording will be a good idea to avoid nits!
+ ret);
+
/* initialize event thread */
priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id;
kthread_init_worker(>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,
>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,
);
+SCHED_FIFO, );
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
[Freedreno] [PATCH v3] drm/msm: validate display and event threads
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
Signed-off-by: Jeykumar Sankaran
---
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,
>disp_thread[i].worker,
"crtc_commit:%d", priv->disp_thread[i].crtc_id);
- ret = sched_setscheduler(priv->disp_thread[i].thread,
- SCHED_FIFO, );
- 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, );
+ if (ret)
+ dev_warn(dev, "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(>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,
>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, );
+SCHED_FIFO, );
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",
+ 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
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
