This is the next part of ipp improvement patches, this time made by YoungJun Cho, I am posting them as he is on leave.
The patchset is based on drm-exynos/exynos-drm-next branch.
Regards Andrzej
Andrzej Hajda (1): drm/exynos: ipp: remove description of non-existing field
YoungJun Cho (8): drm/exynos: ipp: remove usless list_empty() functions drm/exynos: ipp: remove duplicated setting drm/exynos: ipp: rename cmd_lock to lock drm/exynos: ipp: add cmd_lock for cmd_list drm/exynos: ipp: add ipp_remove_id() drm/exynos: ipp: rearrange c_node->mem_lock using routines drm/exynos: ipp: rearrange c_node->event_lock using routine drm/exynos: ipp: update comment for struct drm_ipp_buf_info
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 242 ++++++++++++++++---------------- drivers/gpu/drm/exynos/exynos_drm_ipp.h | 9 +- 2 files changed, 129 insertions(+), 122 deletions(-)
From: YoungJun Cho yj44.cho@samsung.com
list_for_each_entry() handles empty lists, so there is no need to check whether the list is empty first.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 61 +++++---------------------------- 1 file changed, 9 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index bf71d97..c8cfa24 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -276,11 +276,6 @@ static struct exynos_drm_ippdrv *ipp_find_drv_by_handle(u32 prop_id)
DRM_DEBUG_KMS("prop_id[%d]\n", prop_id);
- if (list_empty(&exynos_drm_ippdrv_list)) { - DRM_DEBUG_KMS("ippdrv_list is empty.\n"); - return ERR_PTR(-ENODEV); - } - /* * This case is search ipp driver by prop_id handle. * sometimes, ipp subsystem find driver by prop_id. @@ -289,11 +284,9 @@ static struct exynos_drm_ippdrv *ipp_find_drv_by_handle(u32 prop_id) list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) { DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n", count++, (int)ippdrv);
- if (!list_empty(&ippdrv->cmd_list)) { - list_for_each_entry(c_node, &ippdrv->cmd_list, list) - if (c_node->property.prop_id == prop_id) - return ippdrv; - } + list_for_each_entry(c_node, &ippdrv->cmd_list, list) + if (c_node->property.prop_id == prop_id) + return ippdrv; }
return ERR_PTR(-ENODEV); @@ -573,11 +566,6 @@ static int ipp_check_mem_list(struct drm_exynos_ipp_cmd_node *c_node) /* source/destination memory list */ head = &c_node->mem_list[i];
- if (list_empty(head)) { - DRM_DEBUG_KMS("%s memory empty.\n", i ? "dst" : "src"); - continue; - } - /* find memory node entry */ list_for_each_entry(m_node, head, list) { DRM_DEBUG_KMS("%s,count[%d]m_node[0x%x]\n", @@ -816,11 +804,6 @@ static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node, struct drm_exynos_ipp_send_event *e, *te; int count = 0;
- if (list_empty(&c_node->event_list)) { - DRM_DEBUG_KMS("event_list is empty.\n"); - return; - } - list_for_each_entry_safe(e, te, &c_node->event_list, base.link) { DRM_DEBUG_KMS("count[%d]e[0x%x]\n", count++, (int)e);
@@ -918,14 +901,12 @@ static void ipp_clean_queue_buf(struct drm_device *drm_dev, { struct drm_exynos_ipp_mem_node *m_node, *tm_node;
- if (!list_empty(&c_node->mem_list[qbuf->ops_id])) { - /* delete list */ - list_for_each_entry_safe(m_node, tm_node, - &c_node->mem_list[qbuf->ops_id], list) { - if (m_node->buf_id == qbuf->buf_id && - m_node->ops_id == qbuf->ops_id) - ipp_put_mem_node(drm_dev, c_node, m_node); - } + /* delete list */ + list_for_each_entry_safe(m_node, tm_node, + &c_node->mem_list[qbuf->ops_id], list) { + if (m_node->buf_id == qbuf->buf_id && + m_node->ops_id == qbuf->ops_id) + ipp_put_mem_node(drm_dev, c_node, m_node); } }
@@ -1361,11 +1342,6 @@ static int ipp_stop_property(struct drm_device *drm_dev, /* source/destination memory list */ head = &c_node->mem_list[i];
- if (list_empty(head)) { - DRM_DEBUG_KMS("mem_list is empty.\n"); - break; - } - list_for_each_entry_safe(m_node, tm_node, head, list) { ret = ipp_put_mem_node(drm_dev, c_node, @@ -1381,11 +1357,6 @@ static int ipp_stop_property(struct drm_device *drm_dev, /* destination memory list */ head = &c_node->mem_list[EXYNOS_DRM_OPS_DST];
- if (list_empty(head)) { - DRM_DEBUG_KMS("mem_list is empty.\n"); - break; - } - list_for_each_entry_safe(m_node, tm_node, head, list) { ret = ipp_put_mem_node(drm_dev, c_node, m_node); if (ret) { @@ -1398,11 +1369,6 @@ static int ipp_stop_property(struct drm_device *drm_dev, /* source memory list */ head = &c_node->mem_list[EXYNOS_DRM_OPS_SRC];
- if (list_empty(head)) { - DRM_DEBUG_KMS("mem_list is empty.\n"); - break; - } - list_for_each_entry_safe(m_node, tm_node, head, list) { ret = ipp_put_mem_node(drm_dev, c_node, m_node); if (ret) { @@ -1790,15 +1756,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev,
DRM_DEBUG_KMS("for priv[0x%x]\n", (int)priv);
- if (list_empty(&exynos_drm_ippdrv_list)) { - DRM_DEBUG_KMS("ippdrv_list is empty.\n"); - goto err_clear; - } - list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) { - if (list_empty(&ippdrv->cmd_list)) - continue; - list_for_each_entry_safe(c_node, tc_node, &ippdrv->cmd_list, list) { DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n", @@ -1825,7 +1783,6 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev, } }
-err_clear: kfree(priv); return; }
From: YoungJun Cho yj44.cho@samsung.com
This patch removes duplicated setting.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.cho@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index c8cfa24..0968777 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -1090,12 +1090,12 @@ int exynos_drm_ipp_cmd_ctrl(struct drm_device *drm_dev, void *data, case IPP_CTRL_PLAY: if (pm_runtime_suspended(ippdrv->dev)) pm_runtime_get_sync(ippdrv->dev); + c_node->state = IPP_STATE_START;
cmd_work = c_node->start_work; cmd_work->ctrl = cmd_ctrl->ctrl; ipp_handle_cmd_work(dev, ippdrv, cmd_work, c_node); - c_node->state = IPP_STATE_START; break; case IPP_CTRL_STOP: cmd_work = c_node->stop_work;
From: YoungJun Cho yj44.cho@samsung.com
The ippdrv->cmd_list requires cmd_lock. So renames cmd_lock to lock for context.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 12 ++++++------ drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 0968777..0d85433 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -507,7 +507,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data, goto err_free_stop; }
- mutex_init(&c_node->cmd_lock); + mutex_init(&c_node->lock); mutex_init(&c_node->mem_lock); mutex_init(&c_node->event_lock);
@@ -542,7 +542,7 @@ static void ipp_clean_cmd_node(struct drm_exynos_ipp_cmd_node *c_node) list_del(&c_node->list);
/* destroy mutex */ - mutex_destroy(&c_node->cmd_lock); + mutex_destroy(&c_node->lock); mutex_destroy(&c_node->mem_lock); mutex_destroy(&c_node->event_lock);
@@ -979,7 +979,7 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, void *data, } break; case IPP_BUF_DEQUEUE: - mutex_lock(&c_node->cmd_lock); + mutex_lock(&c_node->lock);
/* put event for destination buffer */ if (qbuf->ops_id == EXYNOS_DRM_OPS_DST) @@ -987,7 +987,7 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, void *data,
ipp_clean_queue_buf(drm_dev, c_node, qbuf);
- mutex_unlock(&c_node->cmd_lock); + mutex_unlock(&c_node->lock); break; default: DRM_ERROR("invalid buffer control.\n"); @@ -1412,7 +1412,7 @@ void ipp_sched_cmd(struct work_struct *work) return; }
- mutex_lock(&c_node->cmd_lock); + mutex_lock(&c_node->lock);
property = &c_node->property;
@@ -1460,7 +1460,7 @@ void ipp_sched_cmd(struct work_struct *work) DRM_DEBUG_KMS("ctrl[%d] done.\n", cmd_work->ctrl);
err_unlock: - mutex_unlock(&c_node->cmd_lock); + mutex_unlock(&c_node->lock); }
static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h index eea4db3..fbb80ac 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h @@ -52,7 +52,7 @@ struct drm_exynos_ipp_cmd_work { * @list: list head to command queue information. * @event_list: list head of event. * @mem_list: list head to source,destination memory queue information. - * @cmd_lock: lock for synchronization of access to ioctl. + * @lock: lock for synchronization of access to ioctl. * @mem_lock: lock for synchronization of access to memory nodes. * @event_lock: lock for synchronization of access to scheduled event. * @start_complete: completion of start of command. @@ -68,7 +68,7 @@ struct drm_exynos_ipp_cmd_node { struct list_head list; struct list_head event_list; struct list_head mem_list[EXYNOS_DRM_OPS_MAX]; - struct mutex cmd_lock; + struct mutex lock; struct mutex mem_lock; struct mutex event_lock; struct completion start_complete;
From: YoungJun Cho yj44.cho@samsung.com
This patch adds cmd_lock for cmd_list synchronization.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 20 ++++++++++++++++++-- drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++ 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 0d85433..b60ae54 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -284,9 +284,14 @@ static struct exynos_drm_ippdrv *ipp_find_drv_by_handle(u32 prop_id) list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) { DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n", count++, (int)ippdrv);
- list_for_each_entry(c_node, &ippdrv->cmd_list, list) - if (c_node->property.prop_id == prop_id) + mutex_lock(&ippdrv->cmd_lock); + list_for_each_entry(c_node, &ippdrv->cmd_list, list) { + if (c_node->property.prop_id == prop_id) { + mutex_unlock(&ippdrv->cmd_lock); return ippdrv; + } + } + mutex_unlock(&ippdrv->cmd_lock); }
return ERR_PTR(-ENODEV); @@ -318,6 +323,7 @@ int exynos_drm_ipp_get_property(struct drm_device *drm_dev, void *data, if (!prop_list->ipp_id) { list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) count++; + /* * Supports ippdrv list count for user application. * First step user application getting ippdrv count. @@ -379,9 +385,11 @@ static int ipp_find_and_set_property(struct drm_exynos_ipp_property *property) * when we find this command no using prop_id. * return property information set in this command node. */ + mutex_lock(&ippdrv->cmd_lock); list_for_each_entry(c_node, &ippdrv->cmd_list, list) { if ((c_node->property.prop_id == prop_id) && (c_node->state == IPP_STATE_STOP)) { + mutex_unlock(&ippdrv->cmd_lock); DRM_DEBUG_KMS("found cmd[%d]ippdrv[0x%x]\n", property->cmd, (int)ippdrv);
@@ -389,6 +397,7 @@ static int ipp_find_and_set_property(struct drm_exynos_ipp_property *property) return 0; } } + mutex_unlock(&ippdrv->cmd_lock);
DRM_ERROR("failed to search property.\n");
@@ -519,7 +528,9 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
INIT_LIST_HEAD(&c_node->event_list); list_splice_init(&priv->event_list, &c_node->event_list); + mutex_lock(&ippdrv->cmd_lock); list_add_tail(&c_node->list, &ippdrv->cmd_list); + mutex_unlock(&ippdrv->cmd_lock);
/* make dedicated state without m2m */ if (!ipp_is_m2m_cmd(property->cmd)) @@ -1110,10 +1121,12 @@ int exynos_drm_ipp_cmd_ctrl(struct drm_device *drm_dev, void *data,
c_node->state = IPP_STATE_STOP; ippdrv->dedicated = false; + mutex_lock(&ippdrv->cmd_lock); ipp_clean_cmd_node(c_node);
if (list_empty(&ippdrv->cmd_list)) pm_runtime_put_sync(ippdrv->dev); + mutex_unlock(&ippdrv->cmd_lock); break; case IPP_CTRL_PAUSE: cmd_work = c_node->stop_work; @@ -1688,6 +1701,7 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev, struct device *dev) ippdrv->event_workq = ctx->event_workq; ippdrv->sched_event = ipp_sched_event; INIT_LIST_HEAD(&ippdrv->cmd_list); + mutex_init(&ippdrv->cmd_lock);
if (is_drm_iommu_supported(drm_dev)) { ret = drm_iommu_attach_device(drm_dev, ippdrv->dev); @@ -1757,6 +1771,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev, DRM_DEBUG_KMS("for priv[0x%x]\n", (int)priv);
list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) { + mutex_lock(&ippdrv->cmd_lock); list_for_each_entry_safe(c_node, tc_node, &ippdrv->cmd_list, list) { DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n", @@ -1781,6 +1796,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev, pm_runtime_put_sync(ippdrv->dev); } } + mutex_unlock(&ippdrv->cmd_lock); }
kfree(priv); diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h index fbb80ac..09cb5a2 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h @@ -148,6 +148,7 @@ struct exynos_drm_ipp_ops { * @event_workq: event work queue. * @c_node: current command information. * @cmd_list: list head for command information. + * @cmd_lock: lock for synchronization of access to cmd_list. * @prop_list: property informations of current ipp driver. * @check_property: check property about format, size, buffer. * @reset: reset ipp block. @@ -165,6 +166,7 @@ struct exynos_drm_ippdrv { struct workqueue_struct *event_workq; struct drm_exynos_ipp_cmd_node *c_node; struct list_head cmd_list; + struct mutex cmd_lock; struct drm_exynos_ipp_prop_list prop_list;
int (*check_property)(struct device *dev,
From: YoungJun Cho yj44.cho@samsung.com
This patch adds ipp_remove_id() for idr resource free.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 42 ++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index b60ae54..f1c51b4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -167,6 +167,13 @@ static int ipp_create_id(struct idr *id_idr, struct mutex *lock, void *obj, return 0; }
+static void ipp_remove_id(struct idr *id_idr, struct mutex *lock, u32 id) +{ + mutex_lock(lock); + idr_remove(id_idr, id); + mutex_unlock(lock); +} + static void *ipp_find_obj(struct idr *id_idr, struct mutex *lock, u32 id) { void *obj; @@ -501,7 +508,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data, c_node->start_work = ipp_create_cmd_work(); if (IS_ERR(c_node->start_work)) { DRM_ERROR("failed to create start work.\n"); - goto err_clear; + goto err_remove_id; }
c_node->stop_work = ipp_create_cmd_work(); @@ -542,16 +549,22 @@ err_free_stop: kfree(c_node->stop_work); err_free_start: kfree(c_node->start_work); +err_remove_id: + ipp_remove_id(&ctx->prop_idr, &ctx->prop_lock, property->prop_id); err_clear: kfree(c_node); return ret; }
-static void ipp_clean_cmd_node(struct drm_exynos_ipp_cmd_node *c_node) +static void ipp_clean_cmd_node(struct ipp_context *ctx, + struct drm_exynos_ipp_cmd_node *c_node) { /* delete list */ list_del(&c_node->list);
+ ipp_remove_id(&ctx->prop_idr, &ctx->prop_lock, + c_node->property.prop_id); + /* destroy mutex */ mutex_destroy(&c_node->lock); mutex_destroy(&c_node->mem_lock); @@ -1122,7 +1135,7 @@ int exynos_drm_ipp_cmd_ctrl(struct drm_device *drm_dev, void *data, c_node->state = IPP_STATE_STOP; ippdrv->dedicated = false; mutex_lock(&ippdrv->cmd_lock); - ipp_clean_cmd_node(c_node); + ipp_clean_cmd_node(ctx, c_node);
if (list_empty(&ippdrv->cmd_list)) pm_runtime_put_sync(ippdrv->dev); @@ -1686,7 +1699,7 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev, struct device *dev) &ipp_id); if (ret || ipp_id == 0) { DRM_ERROR("failed to create id.\n"); - goto err_idr; + goto err; }
DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]ipp_id[%d]\n", @@ -1707,34 +1720,40 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev, struct device *dev) ret = drm_iommu_attach_device(drm_dev, ippdrv->dev); if (ret) { DRM_ERROR("failed to activate iommu\n"); - goto err_iommu; + goto err; } } }
return 0;
-err_iommu: +err: /* get ipp driver entry */ - list_for_each_entry_reverse(ippdrv, &exynos_drm_ippdrv_list, drv_list) + list_for_each_entry_continue_reverse(ippdrv, &exynos_drm_ippdrv_list, + drv_list) { if (is_drm_iommu_supported(drm_dev)) drm_iommu_detach_device(drm_dev, ippdrv->dev);
-err_idr: - idr_destroy(&ctx->ipp_idr); - idr_destroy(&ctx->prop_idr); + ipp_remove_id(&ctx->ipp_idr, &ctx->ipp_lock, + ippdrv->prop_list.ipp_id); + } + return ret; }
static void ipp_subdrv_remove(struct drm_device *drm_dev, struct device *dev) { struct exynos_drm_ippdrv *ippdrv; + struct ipp_context *ctx = get_ipp_context(dev);
/* get ipp driver entry */ list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) { if (is_drm_iommu_supported(drm_dev)) drm_iommu_detach_device(drm_dev, ippdrv->dev);
+ ipp_remove_id(&ctx->ipp_idr, &ctx->ipp_lock, + ippdrv->prop_list.ipp_id); + ippdrv->drm_dev = NULL; exynos_drm_ippdrv_unregister(ippdrv); } @@ -1765,6 +1784,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev, struct drm_exynos_file_private *file_priv = file->driver_priv; struct exynos_drm_ipp_private *priv = file_priv->ipp_priv; struct exynos_drm_ippdrv *ippdrv = NULL; + struct ipp_context *ctx = get_ipp_context(dev); struct drm_exynos_ipp_cmd_node *c_node, *tc_node; int count = 0;
@@ -1791,7 +1811,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev, }
ippdrv->dedicated = false; - ipp_clean_cmd_node(c_node); + ipp_clean_cmd_node(ctx, c_node); if (list_empty(&ippdrv->cmd_list)) pm_runtime_put_sync(ippdrv->dev); }
From: YoungJun Cho yj44.cho@samsung.com
The c_node->mem_list[] should be protected with c_node->mem_lock.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 86 ++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index f1c51b4..4b5afd3 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -584,8 +584,6 @@ static int ipp_check_mem_list(struct drm_exynos_ipp_cmd_node *c_node) struct list_head *head; int ret, i, count[EXYNOS_DRM_OPS_MAX] = { 0, };
- mutex_lock(&c_node->mem_lock); - for_each_ipp_ops(i) { /* source/destination memory list */ head = &c_node->mem_list[i]; @@ -614,8 +612,6 @@ static int ipp_check_mem_list(struct drm_exynos_ipp_cmd_node *c_node) ret = max(count[EXYNOS_DRM_OPS_SRC], count[EXYNOS_DRM_OPS_DST]);
- mutex_unlock(&c_node->mem_lock); - return ret; }
@@ -658,16 +654,13 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv, return -EFAULT; }
- mutex_lock(&c_node->mem_lock); - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
/* get operations callback */ ops = ippdrv->ops[m_node->ops_id]; if (!ops) { DRM_ERROR("not support ops.\n"); - ret = -EFAULT; - goto err_unlock; + return -EFAULT; }
/* set address and enable irq */ @@ -676,12 +669,10 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv, m_node->buf_id, IPP_BUF_ENQUEUE); if (ret) { DRM_ERROR("failed to set addr.\n"); - goto err_unlock; + return ret; } }
-err_unlock: - mutex_unlock(&c_node->mem_lock); return ret; }
@@ -696,11 +687,9 @@ static struct drm_exynos_ipp_mem_node void *addr; int i;
- mutex_lock(&c_node->mem_lock); - m_node = kzalloc(sizeof(*m_node), GFP_KERNEL); if (!m_node) - goto err_unlock; + return ERR_PTR(-ENOMEM);
/* clear base address for error handling */ memset(&buf_info, 0x0, sizeof(buf_info)); @@ -734,15 +723,14 @@ static struct drm_exynos_ipp_mem_node
m_node->filp = file; m_node->buf_info = buf_info; + mutex_lock(&c_node->mem_lock); list_add_tail(&m_node->list, &c_node->mem_list[qbuf->ops_id]); - mutex_unlock(&c_node->mem_lock); + return m_node;
err_clear: kfree(m_node); -err_unlock: - mutex_unlock(&c_node->mem_lock); return ERR_PTR(-EFAULT); }
@@ -759,13 +747,6 @@ static int ipp_put_mem_node(struct drm_device *drm_dev, return -EFAULT; }
- if (list_empty(&m_node->list)) { - DRM_ERROR("empty memory node.\n"); - return -ENOMEM; - } - - mutex_lock(&c_node->mem_lock); - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
/* put gem buffer */ @@ -780,8 +761,6 @@ static int ipp_put_mem_node(struct drm_device *drm_dev, list_del(&m_node->list); kfree(m_node);
- mutex_unlock(&c_node->mem_lock); - return 0; }
@@ -894,7 +873,9 @@ static int ipp_queue_buf_with_run(struct device *dev, return 0; }
+ mutex_lock(&c_node->mem_lock); if (!ipp_check_mem_list(c_node)) { + mutex_unlock(&c_node->mem_lock); DRM_DEBUG_KMS("empty memory.\n"); return 0; } @@ -911,10 +892,12 @@ static int ipp_queue_buf_with_run(struct device *dev, } else { ret = ipp_set_mem_node(ippdrv, c_node, m_node); if (ret) { + mutex_unlock(&c_node->mem_lock); DRM_ERROR("failed to set m node.\n"); return ret; } } + mutex_unlock(&c_node->mem_lock);
return 0; } @@ -926,12 +909,14 @@ static void ipp_clean_queue_buf(struct drm_device *drm_dev, struct drm_exynos_ipp_mem_node *m_node, *tm_node;
/* delete list */ + mutex_lock(&c_node->mem_lock); list_for_each_entry_safe(m_node, tm_node, &c_node->mem_list[qbuf->ops_id], list) { if (m_node->buf_id == qbuf->buf_id && m_node->ops_id == qbuf->ops_id) ipp_put_mem_node(drm_dev, c_node, m_node); } + mutex_unlock(&c_node->mem_lock); }
int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, void *data, @@ -1267,9 +1252,11 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, /* store command info in ippdrv */ ippdrv->c_node = c_node;
+ mutex_lock(&c_node->mem_lock); if (!ipp_check_mem_list(c_node)) { DRM_DEBUG_KMS("empty memory.\n"); - return -ENOMEM; + ret = -ENOMEM; + goto err_unlock; }
/* set current property in ippdrv */ @@ -1277,7 +1264,7 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, if (ret) { DRM_ERROR("failed to set property.\n"); ippdrv->c_node = NULL; - return ret; + goto err_unlock; }
/* check command */ @@ -1292,7 +1279,7 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, if (!m_node) { DRM_ERROR("failed to get node.\n"); ret = -EFAULT; - return ret; + goto err_unlock; }
DRM_DEBUG_KMS("m_node[0x%x]\n", (int)m_node); @@ -1300,7 +1287,7 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, ret = ipp_set_mem_node(ippdrv, c_node, m_node); if (ret) { DRM_ERROR("failed to set m node.\n"); - return ret; + goto err_unlock; } } break; @@ -1312,7 +1299,7 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, ret = ipp_set_mem_node(ippdrv, c_node, m_node); if (ret) { DRM_ERROR("failed to set m node.\n"); - return ret; + goto err_unlock; } } break; @@ -1324,14 +1311,16 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, ret = ipp_set_mem_node(ippdrv, c_node, m_node); if (ret) { DRM_ERROR("failed to set m node.\n"); - return ret; + goto err_unlock; } } break; default: DRM_ERROR("invalid operations.\n"); - return -EINVAL; + ret = -EINVAL; + goto err_unlock; } + mutex_unlock(&c_node->mem_lock);
DRM_DEBUG_KMS("cmd[%d]\n", property->cmd);
@@ -1340,11 +1329,17 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv, ret = ippdrv->start(ippdrv->dev, property->cmd); if (ret) { DRM_ERROR("failed to start ops.\n"); + ippdrv->c_node = NULL; return ret; } }
return 0; + +err_unlock: + mutex_unlock(&c_node->mem_lock); + ippdrv->c_node = NULL; + return ret; }
static int ipp_stop_property(struct drm_device *drm_dev, @@ -1361,6 +1356,8 @@ static int ipp_stop_property(struct drm_device *drm_dev, /* put event */ ipp_put_event(c_node, NULL);
+ mutex_lock(&c_node->mem_lock); + /* check command */ switch (property->cmd) { case IPP_CMD_M2M: @@ -1410,6 +1407,8 @@ static int ipp_stop_property(struct drm_device *drm_dev, }
err_clear: + mutex_unlock(&c_node->mem_lock); + /* stop operations */ if (ippdrv->stop) ippdrv->stop(ippdrv->dev, property->cmd); @@ -1521,9 +1520,11 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, return 0; }
+ mutex_lock(&c_node->mem_lock); if (!ipp_check_mem_list(c_node)) { DRM_DEBUG_KMS("empty memory.\n"); - return 0; + ret = 0; + goto err_mem_unlock; }
/* check command */ @@ -1537,7 +1538,8 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, struct drm_exynos_ipp_mem_node, list); if (!m_node) { DRM_ERROR("empty memory node.\n"); - return -ENOMEM; + ret = -ENOMEM; + goto err_mem_unlock; }
tbuf_id[i] = m_node->buf_id; @@ -1559,7 +1561,8 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, m_node = ipp_find_mem_node(c_node, &qbuf); if (!m_node) { DRM_ERROR("empty memory node.\n"); - return -ENOMEM; + ret = -ENOMEM; + goto err_mem_unlock; }
tbuf_id[EXYNOS_DRM_OPS_DST] = m_node->buf_id; @@ -1576,7 +1579,8 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, struct drm_exynos_ipp_mem_node, list); if (!m_node) { DRM_ERROR("empty memory node.\n"); - return -ENOMEM; + ret = -ENOMEM; + goto err_mem_unlock; }
tbuf_id[EXYNOS_DRM_OPS_SRC] = m_node->buf_id; @@ -1587,8 +1591,10 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, break; default: DRM_ERROR("invalid operations.\n"); - return -EINVAL; + ret = -EINVAL; + goto err_mem_unlock; } + mutex_unlock(&c_node->mem_lock);
if (tbuf_id[EXYNOS_DRM_OPS_DST] != buf_id[EXYNOS_DRM_OPS_DST]) DRM_ERROR("failed to match buf_id[%d %d]prop_id[%d]\n", @@ -1627,6 +1633,10 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, property->cmd, property->prop_id, tbuf_id[EXYNOS_DRM_OPS_DST]);
return 0; + +err_mem_unlock: + mutex_unlock(&c_node->mem_lock); + return ret; }
void ipp_sched_event(struct work_struct *work)
From: YoungJun Cho yj44.cho@samsung.com
The c_node->event_list should be protected with c_node->event_lock.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c index 4b5afd3..603a796 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c @@ -796,7 +796,9 @@ static int ipp_get_event(struct drm_device *drm_dev, e->base.event = &e->event.base; e->base.file_priv = file; e->base.destroy = ipp_free_event; + mutex_lock(&c_node->event_lock); list_add_tail(&e->base.link, &c_node->event_list); + mutex_unlock(&c_node->event_lock);
return 0; } @@ -807,6 +809,7 @@ static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node, struct drm_exynos_ipp_send_event *e, *te; int count = 0;
+ mutex_lock(&c_node->event_lock); list_for_each_entry_safe(e, te, &c_node->event_list, base.link) { DRM_DEBUG_KMS("count[%d]e[0x%x]\n", count++, (int)e);
@@ -827,9 +830,13 @@ static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node, /* delete list */ list_del(&e->base.link); kfree(e); - return; + goto out_unlock; } } + +out_unlock: + mutex_unlock(&c_node->event_lock); + return; }
static void ipp_handle_cmd_work(struct device *dev, @@ -1515,9 +1522,11 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, return -EINVAL; }
+ mutex_lock(&c_node->event_lock); if (list_empty(&c_node->event_list)) { DRM_DEBUG_KMS("event list is empty.\n"); - return 0; + ret = 0; + goto err_event_unlock; }
mutex_lock(&c_node->mem_lock); @@ -1609,11 +1618,6 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, e = list_first_entry(&c_node->event_list, struct drm_exynos_ipp_send_event, base.link);
- if (!e) { - DRM_ERROR("empty event.\n"); - return -EINVAL; - } - do_gettimeofday(&now); DRM_DEBUG_KMS("tv_sec[%ld]tv_usec[%ld]\n", now.tv_sec, now.tv_usec); e->event.tv_sec = now.tv_sec; @@ -1628,6 +1632,7 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv, list_move_tail(&e->base.link, &e->base.file_priv->event_list); wake_up_interruptible(&e->base.file_priv->event_wait); spin_unlock_irqrestore(&drm_dev->event_lock, flags); + mutex_unlock(&c_node->event_lock);
DRM_DEBUG_KMS("done cmd[%d]prop_id[%d]buf_id[%d]\n", property->cmd, property->prop_id, tbuf_id[EXYNOS_DRM_OPS_DST]); @@ -1636,6 +1641,8 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv,
err_mem_unlock: mutex_unlock(&c_node->mem_lock); +err_event_unlock: + mutex_unlock(&c_node->event_lock); return ret; }
@@ -1678,8 +1685,6 @@ void ipp_sched_event(struct work_struct *work) goto err_completion; }
- mutex_lock(&c_node->event_lock); - ret = ipp_send_event(ippdrv, c_node, event_work->buf_id); if (ret) { DRM_ERROR("failed to send event.\n"); @@ -1689,8 +1694,6 @@ void ipp_sched_event(struct work_struct *work) err_completion: if (ipp_is_m2m_cmd(c_node->property.cmd)) complete(&c_node->start_complete); - - mutex_unlock(&c_node->event_lock); }
static int ipp_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
From: YoungJun Cho yj44.cho@samsung.com
The attribute gem_objs in struct drm_exynos_ipp_buf_info was changed to handles. So the comment needs to be updated also.
Signed-off-by: YoungJun Cho yj44.cho@samsung.com Acked-by: Seong-Woo Kim sw0312.kim@samsung.com Acked-by: Kyungmin Park kyungmin.park@samsung.com Tested-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h index 09cb5a2..e06c41e 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h @@ -83,7 +83,7 @@ struct drm_exynos_ipp_cmd_node { /* * A structure of buffer information. * - * @gem_objs: Y, Cb, Cr each gem object. + * @handles: Y, Cb, Cr each gem object handle. * @base: Y, Cb, Cr each planar address. */ struct drm_exynos_ipp_buf_info {
ipp_id field is removed from exynos_drm_ippdrv struct. The patch removes its description as well.
Signed-off-by: Andrzej Hajda a.hajda@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_ipp.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h index e06c41e..7aaeaae 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h @@ -142,7 +142,6 @@ struct exynos_drm_ipp_ops { * @parent_dev: parent device information. * @dev: platform device. * @drm_dev: drm device. - * @ipp_id: id of ipp driver. * @dedicated: dedicated ipp device. * @ops: source, destination operations. * @event_workq: event work queue.
dri-devel@lists.freedesktop.org
-
Andrzej Hajda