* [PATCH 00/15] drm/exynos/ipp: image post processing fixes and improvements, part four
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
This set of patches contains various improvement and fixes
for exynos_drm ipp framework.
The patchset is based on exynos-drm-next branch.
IPP framework was tested for regressions on exynos4210-trats target.
Regards
Andrzej
Andrzej Hajda (15):
drm/exynos/ipp: remove fake pm callbacks
drm/exynos/ipp: cancel works before command node clean
drm/exynos/ipp: move file reference from memory to command node
drm/exynos/ipp: remove only related commands on file close
drm/exynos/ipp: remove unused field in command node
drm/exynos/ipp: free partially allocated resources on error
drm/exynos/ipp: move nodes cleaning to separate function
drm/exynos/ipp: clean memory nodes on command node cleaning
drm/exynos/ipp: replace work_struct casting with better constructs
drm/exynos/ipp: stop hardware before freeing memory
drm/exynos/ipp: remove events during command cleaning
drm/exynos/fimc: avoid clearing overflow bits
drm/exynos/fimc: do not enable fimc twice
drm/exynos/fimc: simplify buffer queuing
drm/exynos/fimc: fix source buffer registers
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 90 ++-----
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +-
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 369 ++++++++++++----------------
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +-
5 files changed, 180 insertions(+), 289 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 00/15] drm/exynos/ipp: image post processing fixes and improvements, part four
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
This set of patches contains various improvement and fixes
for exynos_drm ipp framework.
The patchset is based on exynos-drm-next branch.
IPP framework was tested for regressions on exynos4210-trats target.
Regards
Andrzej
Andrzej Hajda (15):
drm/exynos/ipp: remove fake pm callbacks
drm/exynos/ipp: cancel works before command node clean
drm/exynos/ipp: move file reference from memory to command node
drm/exynos/ipp: remove only related commands on file close
drm/exynos/ipp: remove unused field in command node
drm/exynos/ipp: free partially allocated resources on error
drm/exynos/ipp: move nodes cleaning to separate function
drm/exynos/ipp: clean memory nodes on command node cleaning
drm/exynos/ipp: replace work_struct casting with better constructs
drm/exynos/ipp: stop hardware before freeing memory
drm/exynos/ipp: remove events during command cleaning
drm/exynos/fimc: avoid clearing overflow bits
drm/exynos/fimc: do not enable fimc twice
drm/exynos/fimc: simplify buffer queuing
drm/exynos/fimc: fix source buffer registers
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 90 ++-----
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +-
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 369 ++++++++++++----------------
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +-
5 files changed, 180 insertions(+), 289 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 01/15] drm/exynos/ipp: remove fake pm callbacks
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
PM callbacks in ipp core do nothing, so the patch removes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 51 ---------------------------------
1 file changed, 51 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c411399..da917ca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1808,63 +1808,12 @@ static int ipp_remove(struct platform_device *pdev)
return 0;
}
-static int ipp_power_ctrl(struct ipp_context *ctx, bool enable)
-{
- DRM_DEBUG_KMS("enable[%d]\n", enable);
-
- return 0;
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int ipp_suspend(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- if (pm_runtime_suspended(dev))
- return 0;
-
- return ipp_power_ctrl(ctx, false);
-}
-
-static int ipp_resume(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- if (!pm_runtime_suspended(dev))
- return ipp_power_ctrl(ctx, true);
-
- return 0;
-}
-#endif
-
-#ifdef CONFIG_PM_RUNTIME
-static int ipp_runtime_suspend(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- return ipp_power_ctrl(ctx, false);
-}
-
-static int ipp_runtime_resume(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- return ipp_power_ctrl(ctx, true);
-}
-#endif
-
-static const struct dev_pm_ops ipp_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(ipp_suspend, ipp_resume)
- SET_RUNTIME_PM_OPS(ipp_runtime_suspend, ipp_runtime_resume, NULL)
-};
-
struct platform_driver ipp_driver = {
.probe = ipp_probe,
.remove = ipp_remove,
.driver = {
.name = "exynos-drm-ipp",
.owner = THIS_MODULE,
- .pm = &ipp_pm_ops,
},
};
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 01/15] drm/exynos/ipp: remove fake pm callbacks
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
PM callbacks in ipp core do nothing, so the patch removes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 51 ---------------------------------
1 file changed, 51 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c411399..da917ca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1808,63 +1808,12 @@ static int ipp_remove(struct platform_device *pdev)
return 0;
}
-static int ipp_power_ctrl(struct ipp_context *ctx, bool enable)
-{
- DRM_DEBUG_KMS("enable[%d]\n", enable);
-
- return 0;
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int ipp_suspend(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- if (pm_runtime_suspended(dev))
- return 0;
-
- return ipp_power_ctrl(ctx, false);
-}
-
-static int ipp_resume(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- if (!pm_runtime_suspended(dev))
- return ipp_power_ctrl(ctx, true);
-
- return 0;
-}
-#endif
-
-#ifdef CONFIG_PM_RUNTIME
-static int ipp_runtime_suspend(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- return ipp_power_ctrl(ctx, false);
-}
-
-static int ipp_runtime_resume(struct device *dev)
-{
- struct ipp_context *ctx = get_ipp_context(dev);
-
- return ipp_power_ctrl(ctx, true);
-}
-#endif
-
-static const struct dev_pm_ops ipp_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(ipp_suspend, ipp_resume)
- SET_RUNTIME_PM_OPS(ipp_runtime_suspend, ipp_runtime_resume, NULL)
-};
-
struct platform_driver ipp_driver = {
.probe = ipp_probe,
.remove = ipp_remove,
.driver = {
.name = "exynos-drm-ipp",
.owner = THIS_MODULE,
- .pm = &ipp_pm_ops,
},
};
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 02/15] drm/exynos/ipp: cancel works before command node clean
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
All pending works should be canceled prior to its removal.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index da917ca..9770966 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -502,6 +502,11 @@ err_clear:
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
+ /* cancel works */
+ cancel_work_sync(&c_node->start_work->work);
+ cancel_work_sync(&c_node->stop_work->work);
+ cancel_work_sync(&c_node->event_work->work);
+
/* delete list */
list_del(&c_node->list);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 02/15] drm/exynos/ipp: cancel works before command node clean
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
All pending works should be canceled prior to its removal.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index da917ca..9770966 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -502,6 +502,11 @@ err_clear:
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
+ /* cancel works */
+ cancel_work_sync(&c_node->start_work->work);
+ cancel_work_sync(&c_node->stop_work->work);
+ cancel_work_sync(&c_node->event_work->work);
+
/* delete list */
list_del(&c_node->list);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
Command node should contain file reference to distinguish commands
created by different processes.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 9770966..bbe9968 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
u32 prop_id;
u32 buf_id;
struct drm_exynos_ipp_buf_info buf_info;
- struct drm_file *filp;
};
/*
@@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
c_node->dev = dev;
c_node->property = *property;
c_node->state = IPP_STATE_IDLE;
+ c_node->filp = file;
c_node->start_work = ipp_create_cmd_work();
if (IS_ERR(c_node->start_work)) {
@@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
}
}
- m_node->filp = file;
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);
@@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
unsigned long handle = m_node->buf_info.handles[i];
if (handle)
exynos_drm_gem_put_dma_addr(drm_dev, handle,
- m_node->filp);
+ c_node->filp);
}
/* delete list in queue */
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index 6f48d62..0311035 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
* @stop_work: stop command work structure.
* @event_work: event work structure.
* @state: state of command node.
+ * @filp: associated file pointer.
*/
struct drm_exynos_ipp_cmd_node {
struct device *dev;
@@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
struct drm_exynos_ipp_cmd_work *stop_work;
struct drm_exynos_ipp_event_work *event_work;
enum drm_exynos_ipp_state state;
+ struct drm_file *filp;
};
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
Command node should contain file reference to distinguish commands
created by different processes.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 9770966..bbe9968 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
u32 prop_id;
u32 buf_id;
struct drm_exynos_ipp_buf_info buf_info;
- struct drm_file *filp;
};
/*
@@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
c_node->dev = dev;
c_node->property = *property;
c_node->state = IPP_STATE_IDLE;
+ c_node->filp = file;
c_node->start_work = ipp_create_cmd_work();
if (IS_ERR(c_node->start_work)) {
@@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
}
}
- m_node->filp = file;
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);
@@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
unsigned long handle = m_node->buf_info.handles[i];
if (handle)
exynos_drm_gem_put_dma_addr(drm_dev, handle,
- m_node->filp);
+ c_node->filp);
}
/* delete list in queue */
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index 6f48d62..0311035 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
* @stop_work: stop command work structure.
* @event_work: event work structure.
* @state: state of command node.
+ * @filp: associated file pointer.
*/
struct drm_exynos_ipp_cmd_node {
struct device *dev;
@@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
struct drm_exynos_ipp_cmd_work *stop_work;
struct drm_exynos_ipp_event_work *event_work;
enum drm_exynos_ipp_state state;
+ struct drm_file *filp;
};
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 04/15] drm/exynos/ipp: remove only related commands on file close
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
On file close driver should remove only command nodes created
via this file.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index bbe9968..81f780e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1681,14 +1681,11 @@ static int ipp_subdrv_open(struct drm_device *drm_dev, struct device *dev,
static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev,
struct drm_file *file)
{
- struct drm_exynos_file_private *file_priv = file->driver_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;
- DRM_DEBUG_KMS("for priv[0x%x]\n", (int)file_priv->ipp_dev);
-
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,
@@ -1696,7 +1693,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev,
DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n",
count++, (int)ippdrv);
- if (c_node->dev == file_priv->ipp_dev) {
+ if (c_node->filp == file) {
/*
* userland goto unnormal state. process killed.
* and close the file.
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 04/15] drm/exynos/ipp: remove only related commands on file close
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
On file close driver should remove only command nodes created
via this file.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index bbe9968..81f780e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1681,14 +1681,11 @@ static int ipp_subdrv_open(struct drm_device *drm_dev, struct device *dev,
static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev,
struct drm_file *file)
{
- struct drm_exynos_file_private *file_priv = file->driver_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;
- DRM_DEBUG_KMS("for priv[0x%x]\n", (int)file_priv->ipp_dev);
-
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,
@@ -1696,7 +1693,7 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev,
DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]\n",
count++, (int)ippdrv);
- if (c_node->dev == file_priv->ipp_dev) {
+ if (c_node->filp == file) {
/*
* userland goto unnormal state. process killed.
* and close the file.
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 05/15] drm/exynos/ipp: remove unused field in command node
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
Since command node have file pointer dev field became useless.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 1 -
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 --
2 files changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 81f780e..060a198 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -444,7 +444,6 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
property->prop_id, property->cmd, (int)ippdrv);
/* stored property information and ippdrv in private data */
- c_node->dev = dev;
c_node->property = *property;
c_node->state = IPP_STATE_IDLE;
c_node->filp = file;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index 0311035..2a61547 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -48,7 +48,6 @@ struct drm_exynos_ipp_cmd_work {
/*
* A structure of command node.
*
- * @dev: IPP device.
* @list: list head to command queue information.
* @event_list: list head of event.
* @mem_list: list head to source,destination memory queue information.
@@ -65,7 +64,6 @@ struct drm_exynos_ipp_cmd_work {
* @filp: associated file pointer.
*/
struct drm_exynos_ipp_cmd_node {
- struct device *dev;
struct list_head list;
struct list_head event_list;
struct list_head mem_list[EXYNOS_DRM_OPS_MAX];
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 05/15] drm/exynos/ipp: remove unused field in command node
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
Since command node have file pointer dev field became useless.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 1 -
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 --
2 files changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 81f780e..060a198 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -444,7 +444,6 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
property->prop_id, property->cmd, (int)ippdrv);
/* stored property information and ippdrv in private data */
- c_node->dev = dev;
c_node->property = *property;
c_node->state = IPP_STATE_IDLE;
c_node->filp = file;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index 0311035..2a61547 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -48,7 +48,6 @@ struct drm_exynos_ipp_cmd_work {
/*
* A structure of command node.
*
- * @dev: IPP device.
* @list: list head to command queue information.
* @event_list: list head of event.
* @mem_list: list head to source,destination memory queue information.
@@ -65,7 +64,6 @@ struct drm_exynos_ipp_cmd_work {
* @filp: associated file pointer.
*/
struct drm_exynos_ipp_cmd_node {
- struct device *dev;
struct list_head list;
struct list_head event_list;
struct list_head mem_list[EXYNOS_DRM_OPS_MAX];
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
In case of allocation errors some already allocated buffers
were not freed. The patch fixes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 060a198..728f3b9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
return ret;
}
+static int ipp_put_mem_node(struct drm_device *drm_dev,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_mem_node *m_node)
+{
+ int i;
+
+ DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
+
+ if (!m_node) {
+ DRM_ERROR("invalid dequeue node.\n");
+ return -EFAULT;
+ }
+
+ DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
+
+ /* put gem buffer */
+ for_each_ipp_planar(i) {
+ unsigned long handle = m_node->buf_info.handles[i];
+ if (handle)
+ exynos_drm_gem_put_dma_addr(drm_dev, handle,
+ c_node->filp);
+ }
+
+ /* conditionally remove from queue */
+ if (m_node->list.next)
+ list_del(&m_node->list);
+ kfree(m_node);
+
+ return 0;
+}
+
static struct drm_exynos_ipp_mem_node
*ipp_get_mem_node(struct drm_device *drm_dev,
struct drm_file *file,
@@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
qbuf->handle[i], file);
if (IS_ERR(addr)) {
DRM_ERROR("failed to get addr.\n");
- goto err_clear;
+ ipp_put_mem_node(drm_dev, c_node, m_node);
+ return ERR_PTR(-EFAULT);
}
buf_info->handles[i] = qbuf->handle[i];
@@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
mutex_unlock(&c_node->mem_lock);
return m_node;
-
-err_clear:
- kfree(m_node);
- return ERR_PTR(-EFAULT);
-}
-
-static int ipp_put_mem_node(struct drm_device *drm_dev,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_mem_node *m_node)
-{
- int i;
-
- DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
-
- if (!m_node) {
- DRM_ERROR("invalid dequeue node.\n");
- return -EFAULT;
- }
-
- DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
-
- /* put gem buffer */
- for_each_ipp_planar(i) {
- unsigned long handle = m_node->buf_info.handles[i];
- if (handle)
- exynos_drm_gem_put_dma_addr(drm_dev, handle,
- c_node->filp);
- }
-
- /* delete list in queue */
- list_del(&m_node->list);
- kfree(m_node);
-
- return 0;
}
static void ipp_free_event(struct drm_pending_event *event)
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
In case of allocation errors some already allocated buffers
were not freed. The patch fixes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 060a198..728f3b9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
return ret;
}
+static int ipp_put_mem_node(struct drm_device *drm_dev,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_mem_node *m_node)
+{
+ int i;
+
+ DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
+
+ if (!m_node) {
+ DRM_ERROR("invalid dequeue node.\n");
+ return -EFAULT;
+ }
+
+ DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
+
+ /* put gem buffer */
+ for_each_ipp_planar(i) {
+ unsigned long handle = m_node->buf_info.handles[i];
+ if (handle)
+ exynos_drm_gem_put_dma_addr(drm_dev, handle,
+ c_node->filp);
+ }
+
+ /* conditionally remove from queue */
+ if (m_node->list.next)
+ list_del(&m_node->list);
+ kfree(m_node);
+
+ return 0;
+}
+
static struct drm_exynos_ipp_mem_node
*ipp_get_mem_node(struct drm_device *drm_dev,
struct drm_file *file,
@@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
qbuf->handle[i], file);
if (IS_ERR(addr)) {
DRM_ERROR("failed to get addr.\n");
- goto err_clear;
+ ipp_put_mem_node(drm_dev, c_node, m_node);
+ return ERR_PTR(-EFAULT);
}
buf_info->handles[i] = qbuf->handle[i];
@@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
mutex_unlock(&c_node->mem_lock);
return m_node;
-
-err_clear:
- kfree(m_node);
- return ERR_PTR(-EFAULT);
-}
-
-static int ipp_put_mem_node(struct drm_device *drm_dev,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_mem_node *m_node)
-{
- int i;
-
- DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
-
- if (!m_node) {
- DRM_ERROR("invalid dequeue node.\n");
- return -EFAULT;
- }
-
- DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
-
- /* put gem buffer */
- for_each_ipp_planar(i) {
- unsigned long handle = m_node->buf_info.handles[i];
- if (handle)
- exynos_drm_gem_put_dma_addr(drm_dev, handle,
- c_node->filp);
- }
-
- /* delete list in queue */
- list_del(&m_node->list);
- kfree(m_node);
-
- return 0;
}
static void ipp_free_event(struct drm_pending_event *event)
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 07/15] drm/exynos/ipp: move nodes cleaning to separate function
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
The patch introduces ipp_clean_mem_nodes function which replaces
redundant code. Additionally memory node function definitions
are moved up to increase its visibility.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 231 +++++++++++++++-----------------
1 file changed, 107 insertions(+), 124 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 728f3b9..22bd551 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -498,6 +498,109 @@ err_clear:
return ret;
}
+static int ipp_put_mem_node(struct drm_device *drm_dev,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_mem_node *m_node)
+{
+ int i;
+
+ DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
+
+ if (!m_node) {
+ DRM_ERROR("invalid dequeue node.\n");
+ return -EFAULT;
+ }
+
+ DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
+
+ /* put gem buffer */
+ for_each_ipp_planar(i) {
+ unsigned long handle = m_node->buf_info.handles[i];
+ if (handle)
+ exynos_drm_gem_put_dma_addr(drm_dev, handle,
+ c_node->filp);
+ }
+
+ /* conditionally remove from queue */
+ if (m_node->list.next)
+ list_del(&m_node->list);
+ kfree(m_node);
+
+ return 0;
+}
+
+static struct drm_exynos_ipp_mem_node
+ *ipp_get_mem_node(struct drm_device *drm_dev,
+ struct drm_file *file,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_queue_buf *qbuf)
+{
+ struct drm_exynos_ipp_mem_node *m_node;
+ struct drm_exynos_ipp_buf_info *buf_info;
+ int i;
+
+ m_node = kzalloc(sizeof(*m_node), GFP_KERNEL);
+ if (!m_node)
+ return ERR_PTR(-ENOMEM);
+
+ buf_info = &m_node->buf_info;
+
+ /* operations, buffer id */
+ m_node->ops_id = qbuf->ops_id;
+ m_node->prop_id = qbuf->prop_id;
+ m_node->buf_id = qbuf->buf_id;
+
+ DRM_DEBUG_KMS("m_node[0x%x]ops_id[%d]\n", (int)m_node, qbuf->ops_id);
+ DRM_DEBUG_KMS("prop_id[%d]buf_id[%d]\n", qbuf->prop_id, m_node->buf_id);
+
+ for_each_ipp_planar(i) {
+ DRM_DEBUG_KMS("i[%d]handle[0x%x]\n", i, qbuf->handle[i]);
+
+ /* get dma address by handle */
+ if (qbuf->handle[i]) {
+ dma_addr_t *addr;
+
+ addr = exynos_drm_gem_get_dma_addr(drm_dev,
+ qbuf->handle[i], file);
+ if (IS_ERR(addr)) {
+ DRM_ERROR("failed to get addr.\n");
+ ipp_put_mem_node(drm_dev, c_node, m_node);
+ return ERR_PTR(-EFAULT);
+ }
+
+ buf_info->handles[i] = qbuf->handle[i];
+ buf_info->base[i] = *addr;
+ DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%lx]\n", i,
+ buf_info->base[i], buf_info->handles[i]);
+ }
+ }
+
+ 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;
+}
+
+static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
+ struct drm_exynos_ipp_cmd_node *c_node, int ops)
+{
+ struct drm_exynos_ipp_mem_node *m_node, *tm_node;
+ struct list_head *head = &c_node->mem_list[ops];
+
+ mutex_lock(&c_node->mem_lock);
+
+ list_for_each_entry_safe(m_node, tm_node, head, list) {
+ int ret;
+
+ ret = ipp_put_mem_node(drm_dev, c_node, m_node);
+ if (ret)
+ DRM_ERROR("failed to put m_node.\n");
+ }
+
+ mutex_unlock(&c_node->mem_lock);
+}
+
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
@@ -599,90 +702,6 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
return ret;
}
-static int ipp_put_mem_node(struct drm_device *drm_dev,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_mem_node *m_node)
-{
- int i;
-
- DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
-
- if (!m_node) {
- DRM_ERROR("invalid dequeue node.\n");
- return -EFAULT;
- }
-
- DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
-
- /* put gem buffer */
- for_each_ipp_planar(i) {
- unsigned long handle = m_node->buf_info.handles[i];
- if (handle)
- exynos_drm_gem_put_dma_addr(drm_dev, handle,
- c_node->filp);
- }
-
- /* conditionally remove from queue */
- if (m_node->list.next)
- list_del(&m_node->list);
- kfree(m_node);
-
- return 0;
-}
-
-static struct drm_exynos_ipp_mem_node
- *ipp_get_mem_node(struct drm_device *drm_dev,
- struct drm_file *file,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_queue_buf *qbuf)
-{
- struct drm_exynos_ipp_mem_node *m_node;
- struct drm_exynos_ipp_buf_info *buf_info;
- int i;
-
- m_node = kzalloc(sizeof(*m_node), GFP_KERNEL);
- if (!m_node)
- return ERR_PTR(-ENOMEM);
-
- buf_info = &m_node->buf_info;
-
- /* operations, buffer id */
- m_node->ops_id = qbuf->ops_id;
- m_node->prop_id = qbuf->prop_id;
- m_node->buf_id = qbuf->buf_id;
-
- DRM_DEBUG_KMS("m_node[0x%x]ops_id[%d]\n", (int)m_node, qbuf->ops_id);
- DRM_DEBUG_KMS("prop_id[%d]buf_id[%d]\n", qbuf->prop_id, m_node->buf_id);
-
- for_each_ipp_planar(i) {
- DRM_DEBUG_KMS("i[%d]handle[0x%x]\n", i, qbuf->handle[i]);
-
- /* get dma address by handle */
- if (qbuf->handle[i]) {
- dma_addr_t *addr;
-
- addr = exynos_drm_gem_get_dma_addr(drm_dev,
- qbuf->handle[i], file);
- if (IS_ERR(addr)) {
- DRM_ERROR("failed to get addr.\n");
- ipp_put_mem_node(drm_dev, c_node, m_node);
- return ERR_PTR(-EFAULT);
- }
-
- buf_info->handles[i] = qbuf->handle[i];
- buf_info->base[i] = *addr;
- DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%lx]\n", i,
- buf_info->base[i], buf_info->handles[i]);
- }
- }
-
- 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;
-}
-
static void ipp_free_event(struct drm_pending_event *event)
{
kfree(event);
@@ -1258,9 +1277,7 @@ static int ipp_stop_property(struct drm_device *drm_dev,
struct exynos_drm_ippdrv *ippdrv,
struct drm_exynos_ipp_cmd_node *c_node)
{
- struct drm_exynos_ipp_mem_node *m_node, *tm_node;
struct drm_exynos_ipp_property *property = &c_node->property;
- struct list_head *head;
int ret = 0, i;
DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
@@ -1268,49 +1285,17 @@ 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:
- for_each_ipp_ops(i) {
- /* source/destination memory list */
- head = &c_node->mem_list[i];
-
- list_for_each_entry_safe(m_node, tm_node,
- head, list) {
- ret = ipp_put_mem_node(drm_dev, c_node,
- m_node);
- if (ret) {
- DRM_ERROR("failed to put m_node.\n");
- goto err_clear;
- }
- }
- }
+ for_each_ipp_ops(i)
+ ipp_clean_mem_nodes(drm_dev, c_node, i);
break;
case IPP_CMD_WB:
- /* destination memory list */
- head = &c_node->mem_list[EXYNOS_DRM_OPS_DST];
-
- list_for_each_entry_safe(m_node, tm_node, head, list) {
- ret = ipp_put_mem_node(drm_dev, c_node, m_node);
- if (ret) {
- DRM_ERROR("failed to put m_node.\n");
- goto err_clear;
- }
- }
+ ipp_clean_mem_nodes(drm_dev, c_node, EXYNOS_DRM_OPS_DST);
break;
case IPP_CMD_OUTPUT:
- /* source memory list */
- head = &c_node->mem_list[EXYNOS_DRM_OPS_SRC];
-
- list_for_each_entry_safe(m_node, tm_node, head, list) {
- ret = ipp_put_mem_node(drm_dev, c_node, m_node);
- if (ret) {
- DRM_ERROR("failed to put m_node.\n");
- goto err_clear;
- }
- }
+ ipp_clean_mem_nodes(drm_dev, c_node, EXYNOS_DRM_OPS_SRC);
break;
default:
DRM_ERROR("invalid operations.\n");
@@ -1319,8 +1304,6 @@ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 07/15] drm/exynos/ipp: move nodes cleaning to separate function
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
The patch introduces ipp_clean_mem_nodes function which replaces
redundant code. Additionally memory node function definitions
are moved up to increase its visibility.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 231 +++++++++++++++-----------------
1 file changed, 107 insertions(+), 124 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 728f3b9..22bd551 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -498,6 +498,109 @@ err_clear:
return ret;
}
+static int ipp_put_mem_node(struct drm_device *drm_dev,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_mem_node *m_node)
+{
+ int i;
+
+ DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
+
+ if (!m_node) {
+ DRM_ERROR("invalid dequeue node.\n");
+ return -EFAULT;
+ }
+
+ DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
+
+ /* put gem buffer */
+ for_each_ipp_planar(i) {
+ unsigned long handle = m_node->buf_info.handles[i];
+ if (handle)
+ exynos_drm_gem_put_dma_addr(drm_dev, handle,
+ c_node->filp);
+ }
+
+ /* conditionally remove from queue */
+ if (m_node->list.next)
+ list_del(&m_node->list);
+ kfree(m_node);
+
+ return 0;
+}
+
+static struct drm_exynos_ipp_mem_node
+ *ipp_get_mem_node(struct drm_device *drm_dev,
+ struct drm_file *file,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_queue_buf *qbuf)
+{
+ struct drm_exynos_ipp_mem_node *m_node;
+ struct drm_exynos_ipp_buf_info *buf_info;
+ int i;
+
+ m_node = kzalloc(sizeof(*m_node), GFP_KERNEL);
+ if (!m_node)
+ return ERR_PTR(-ENOMEM);
+
+ buf_info = &m_node->buf_info;
+
+ /* operations, buffer id */
+ m_node->ops_id = qbuf->ops_id;
+ m_node->prop_id = qbuf->prop_id;
+ m_node->buf_id = qbuf->buf_id;
+
+ DRM_DEBUG_KMS("m_node[0x%x]ops_id[%d]\n", (int)m_node, qbuf->ops_id);
+ DRM_DEBUG_KMS("prop_id[%d]buf_id[%d]\n", qbuf->prop_id, m_node->buf_id);
+
+ for_each_ipp_planar(i) {
+ DRM_DEBUG_KMS("i[%d]handle[0x%x]\n", i, qbuf->handle[i]);
+
+ /* get dma address by handle */
+ if (qbuf->handle[i]) {
+ dma_addr_t *addr;
+
+ addr = exynos_drm_gem_get_dma_addr(drm_dev,
+ qbuf->handle[i], file);
+ if (IS_ERR(addr)) {
+ DRM_ERROR("failed to get addr.\n");
+ ipp_put_mem_node(drm_dev, c_node, m_node);
+ return ERR_PTR(-EFAULT);
+ }
+
+ buf_info->handles[i] = qbuf->handle[i];
+ buf_info->base[i] = *addr;
+ DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%lx]\n", i,
+ buf_info->base[i], buf_info->handles[i]);
+ }
+ }
+
+ 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;
+}
+
+static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
+ struct drm_exynos_ipp_cmd_node *c_node, int ops)
+{
+ struct drm_exynos_ipp_mem_node *m_node, *tm_node;
+ struct list_head *head = &c_node->mem_list[ops];
+
+ mutex_lock(&c_node->mem_lock);
+
+ list_for_each_entry_safe(m_node, tm_node, head, list) {
+ int ret;
+
+ ret = ipp_put_mem_node(drm_dev, c_node, m_node);
+ if (ret)
+ DRM_ERROR("failed to put m_node.\n");
+ }
+
+ mutex_unlock(&c_node->mem_lock);
+}
+
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
@@ -599,90 +702,6 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
return ret;
}
-static int ipp_put_mem_node(struct drm_device *drm_dev,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_mem_node *m_node)
-{
- int i;
-
- DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
-
- if (!m_node) {
- DRM_ERROR("invalid dequeue node.\n");
- return -EFAULT;
- }
-
- DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
-
- /* put gem buffer */
- for_each_ipp_planar(i) {
- unsigned long handle = m_node->buf_info.handles[i];
- if (handle)
- exynos_drm_gem_put_dma_addr(drm_dev, handle,
- c_node->filp);
- }
-
- /* conditionally remove from queue */
- if (m_node->list.next)
- list_del(&m_node->list);
- kfree(m_node);
-
- return 0;
-}
-
-static struct drm_exynos_ipp_mem_node
- *ipp_get_mem_node(struct drm_device *drm_dev,
- struct drm_file *file,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_queue_buf *qbuf)
-{
- struct drm_exynos_ipp_mem_node *m_node;
- struct drm_exynos_ipp_buf_info *buf_info;
- int i;
-
- m_node = kzalloc(sizeof(*m_node), GFP_KERNEL);
- if (!m_node)
- return ERR_PTR(-ENOMEM);
-
- buf_info = &m_node->buf_info;
-
- /* operations, buffer id */
- m_node->ops_id = qbuf->ops_id;
- m_node->prop_id = qbuf->prop_id;
- m_node->buf_id = qbuf->buf_id;
-
- DRM_DEBUG_KMS("m_node[0x%x]ops_id[%d]\n", (int)m_node, qbuf->ops_id);
- DRM_DEBUG_KMS("prop_id[%d]buf_id[%d]\n", qbuf->prop_id, m_node->buf_id);
-
- for_each_ipp_planar(i) {
- DRM_DEBUG_KMS("i[%d]handle[0x%x]\n", i, qbuf->handle[i]);
-
- /* get dma address by handle */
- if (qbuf->handle[i]) {
- dma_addr_t *addr;
-
- addr = exynos_drm_gem_get_dma_addr(drm_dev,
- qbuf->handle[i], file);
- if (IS_ERR(addr)) {
- DRM_ERROR("failed to get addr.\n");
- ipp_put_mem_node(drm_dev, c_node, m_node);
- return ERR_PTR(-EFAULT);
- }
-
- buf_info->handles[i] = qbuf->handle[i];
- buf_info->base[i] = *addr;
- DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%lx]\n", i,
- buf_info->base[i], buf_info->handles[i]);
- }
- }
-
- 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;
-}
-
static void ipp_free_event(struct drm_pending_event *event)
{
kfree(event);
@@ -1258,9 +1277,7 @@ static int ipp_stop_property(struct drm_device *drm_dev,
struct exynos_drm_ippdrv *ippdrv,
struct drm_exynos_ipp_cmd_node *c_node)
{
- struct drm_exynos_ipp_mem_node *m_node, *tm_node;
struct drm_exynos_ipp_property *property = &c_node->property;
- struct list_head *head;
int ret = 0, i;
DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
@@ -1268,49 +1285,17 @@ 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:
- for_each_ipp_ops(i) {
- /* source/destination memory list */
- head = &c_node->mem_list[i];
-
- list_for_each_entry_safe(m_node, tm_node,
- head, list) {
- ret = ipp_put_mem_node(drm_dev, c_node,
- m_node);
- if (ret) {
- DRM_ERROR("failed to put m_node.\n");
- goto err_clear;
- }
- }
- }
+ for_each_ipp_ops(i)
+ ipp_clean_mem_nodes(drm_dev, c_node, i);
break;
case IPP_CMD_WB:
- /* destination memory list */
- head = &c_node->mem_list[EXYNOS_DRM_OPS_DST];
-
- list_for_each_entry_safe(m_node, tm_node, head, list) {
- ret = ipp_put_mem_node(drm_dev, c_node, m_node);
- if (ret) {
- DRM_ERROR("failed to put m_node.\n");
- goto err_clear;
- }
- }
+ ipp_clean_mem_nodes(drm_dev, c_node, EXYNOS_DRM_OPS_DST);
break;
case IPP_CMD_OUTPUT:
- /* source memory list */
- head = &c_node->mem_list[EXYNOS_DRM_OPS_SRC];
-
- list_for_each_entry_safe(m_node, tm_node, head, list) {
- ret = ipp_put_mem_node(drm_dev, c_node, m_node);
- if (ret) {
- DRM_ERROR("failed to put m_node.\n");
- goto err_clear;
- }
- }
+ ipp_clean_mem_nodes(drm_dev, c_node, EXYNOS_DRM_OPS_SRC);
break;
default:
DRM_ERROR("invalid operations.\n");
@@ -1319,8 +1304,6 @@ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 08/15] drm/exynos/ipp: clean memory nodes on command node cleaning
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
The nodes should be removed before removing command node.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 22bd551..6ab6190 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -604,11 +604,16 @@ static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
+ int i;
+
/* cancel works */
cancel_work_sync(&c_node->start_work->work);
cancel_work_sync(&c_node->stop_work->work);
cancel_work_sync(&c_node->event_work->work);
+ for_each_ipp_ops(i)
+ ipp_clean_mem_nodes(ctx->subdrv.drm_dev, c_node, i);
+
/* delete list */
list_del(&c_node->list);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 08/15] drm/exynos/ipp: clean memory nodes on command node cleaning
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
The nodes should be removed before removing command node.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 22bd551..6ab6190 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -604,11 +604,16 @@ static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
+ int i;
+
/* cancel works */
cancel_work_sync(&c_node->start_work->work);
cancel_work_sync(&c_node->stop_work->work);
cancel_work_sync(&c_node->event_work->work);
+ for_each_ipp_ops(i)
+ ipp_clean_mem_nodes(ctx->subdrv.drm_dev, c_node, i);
+
/* delete list */
list_del(&c_node->list);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 09/15] drm/exynos/ipp: replace work_struct casting with better constructs
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
Type casting should be avoided if possible. In case of
work_struct it can be simply replaced by reference to member field.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +--
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 6 +++---
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +--
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 35db665..3264ed3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1307,7 +1307,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
- queue_work(ippdrv->event_workq, (struct work_struct *)event_work);
+ queue_work(ippdrv->event_workq, &event_work->work);
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 9e3ff16..c6a013f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1326,8 +1326,7 @@ static irqreturn_t gsc_irq_handler(int irq, void *dev_id)
buf_id[EXYNOS_DRM_OPS_SRC];
event_work->buf_id[EXYNOS_DRM_OPS_DST] =
buf_id[EXYNOS_DRM_OPS_DST];
- queue_work(ippdrv->event_workq,
- (struct work_struct *)event_work);
+ queue_work(ippdrv->event_workq, &event_work->work);
}
return IRQ_HANDLED;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 6ab6190..c72d8d1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -791,7 +791,7 @@ static void ipp_handle_cmd_work(struct device *dev,
cmd_work->ippdrv = ippdrv;
cmd_work->c_node = c_node;
- queue_work(ctx->cmd_workq, (struct work_struct *)cmd_work);
+ queue_work(ctx->cmd_workq, &cmd_work->work);
}
static int ipp_queue_buf_with_run(struct device *dev,
@@ -1319,7 +1319,7 @@ err_clear:
void ipp_sched_cmd(struct work_struct *work)
{
struct drm_exynos_ipp_cmd_work *cmd_work =
- (struct drm_exynos_ipp_cmd_work *)work;
+ container_of(work, struct drm_exynos_ipp_cmd_work, work);
struct exynos_drm_ippdrv *ippdrv;
struct drm_exynos_ipp_cmd_node *c_node;
struct drm_exynos_ipp_property *property;
@@ -1532,7 +1532,7 @@ err_event_unlock:
void ipp_sched_event(struct work_struct *work)
{
struct drm_exynos_ipp_event_work *event_work =
- (struct drm_exynos_ipp_event_work *)work;
+ container_of(work, struct drm_exynos_ipp_event_work, work);
struct exynos_drm_ippdrv *ippdrv;
struct drm_exynos_ipp_cmd_node *c_node;
int ret;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
index 55af6b4..b6a37d4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
@@ -156,8 +156,7 @@ static irqreturn_t rotator_irq_handler(int irq, void *arg)
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] =
rot->cur_buf_id[EXYNOS_DRM_OPS_DST];
- queue_work(ippdrv->event_workq,
- (struct work_struct *)event_work);
+ queue_work(ippdrv->event_workq, &event_work->work);
} else {
DRM_ERROR("the SFR is set illegally\n");
}
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 09/15] drm/exynos/ipp: replace work_struct casting with better constructs
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
Type casting should be avoided if possible. In case of
work_struct it can be simply replaced by reference to member field.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +--
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 6 +++---
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +--
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 35db665..3264ed3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1307,7 +1307,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
- queue_work(ippdrv->event_workq, (struct work_struct *)event_work);
+ queue_work(ippdrv->event_workq, &event_work->work);
return IRQ_HANDLED;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 9e3ff16..c6a013f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -1326,8 +1326,7 @@ static irqreturn_t gsc_irq_handler(int irq, void *dev_id)
buf_id[EXYNOS_DRM_OPS_SRC];
event_work->buf_id[EXYNOS_DRM_OPS_DST] =
buf_id[EXYNOS_DRM_OPS_DST];
- queue_work(ippdrv->event_workq,
- (struct work_struct *)event_work);
+ queue_work(ippdrv->event_workq, &event_work->work);
}
return IRQ_HANDLED;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 6ab6190..c72d8d1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -791,7 +791,7 @@ static void ipp_handle_cmd_work(struct device *dev,
cmd_work->ippdrv = ippdrv;
cmd_work->c_node = c_node;
- queue_work(ctx->cmd_workq, (struct work_struct *)cmd_work);
+ queue_work(ctx->cmd_workq, &cmd_work->work);
}
static int ipp_queue_buf_with_run(struct device *dev,
@@ -1319,7 +1319,7 @@ err_clear:
void ipp_sched_cmd(struct work_struct *work)
{
struct drm_exynos_ipp_cmd_work *cmd_work =
- (struct drm_exynos_ipp_cmd_work *)work;
+ container_of(work, struct drm_exynos_ipp_cmd_work, work);
struct exynos_drm_ippdrv *ippdrv;
struct drm_exynos_ipp_cmd_node *c_node;
struct drm_exynos_ipp_property *property;
@@ -1532,7 +1532,7 @@ err_event_unlock:
void ipp_sched_event(struct work_struct *work)
{
struct drm_exynos_ipp_event_work *event_work =
- (struct drm_exynos_ipp_event_work *)work;
+ container_of(work, struct drm_exynos_ipp_event_work, work);
struct exynos_drm_ippdrv *ippdrv;
struct drm_exynos_ipp_cmd_node *c_node;
int ret;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
index 55af6b4..b6a37d4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
@@ -156,8 +156,7 @@ static irqreturn_t rotator_irq_handler(int irq, void *arg)
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] =
rot->cur_buf_id[EXYNOS_DRM_OPS_DST];
- queue_work(ippdrv->event_workq,
- (struct work_struct *)event_work);
+ queue_work(ippdrv->event_workq, &event_work->work);
} else {
DRM_ERROR("the SFR is set illegally\n");
}
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 10/15] drm/exynos/ipp: stop hardware before freeing memory
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
Memory shouldn't be freed when hardware is still running.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c72d8d1..6de75aa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1283,12 +1283,15 @@ static int ipp_stop_property(struct drm_device *drm_dev,
struct drm_exynos_ipp_cmd_node *c_node)
{
struct drm_exynos_ipp_property *property = &c_node->property;
- int ret = 0, i;
+ int i;
DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
/* put event */
ipp_put_event(c_node, NULL);
+ /* stop operations */
+ if (ippdrv->stop)
+ ippdrv->stop(ippdrv->dev, property->cmd);
/* check command */
switch (property->cmd) {
@@ -1304,16 +1307,10 @@ static int ipp_stop_property(struct drm_device *drm_dev,
break;
default:
DRM_ERROR("invalid operations.\n");
- ret = -EINVAL;
- goto err_clear;
+ return -EINVAL;
}
-err_clear:
- /* stop operations */
- if (ippdrv->stop)
- ippdrv->stop(ippdrv->dev, property->cmd);
-
- return ret;
+ return 0;
}
void ipp_sched_cmd(struct work_struct *work)
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 10/15] drm/exynos/ipp: stop hardware before freeing memory
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
Memory shouldn't be freed when hardware is still running.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c72d8d1..6de75aa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1283,12 +1283,15 @@ static int ipp_stop_property(struct drm_device *drm_dev,
struct drm_exynos_ipp_cmd_node *c_node)
{
struct drm_exynos_ipp_property *property = &c_node->property;
- int ret = 0, i;
+ int i;
DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
/* put event */
ipp_put_event(c_node, NULL);
+ /* stop operations */
+ if (ippdrv->stop)
+ ippdrv->stop(ippdrv->dev, property->cmd);
/* check command */
switch (property->cmd) {
@@ -1304,16 +1307,10 @@ static int ipp_stop_property(struct drm_device *drm_dev,
break;
default:
DRM_ERROR("invalid operations.\n");
- ret = -EINVAL;
- goto err_clear;
+ return -EINVAL;
}
-err_clear:
- /* stop operations */
- if (ippdrv->stop)
- ippdrv->stop(ippdrv->dev, property->cmd);
-
- return ret;
+ return 0;
}
void ipp_sched_cmd(struct work_struct *work)
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 11/15] drm/exynos/ipp: remove events during command cleaning
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
Events were removed only during stop command, as a result
there were memory leaks if program prematurely exited.
This patch fixes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 155 ++++++++++++++++----------------
1 file changed, 78 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 6de75aa..ec88393 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -601,6 +601,81 @@ static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
mutex_unlock(&c_node->mem_lock);
}
+static void ipp_free_event(struct drm_pending_event *event)
+{
+ kfree(event);
+}
+
+static int ipp_get_event(struct drm_device *drm_dev,
+ struct drm_file *file,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_queue_buf *qbuf)
+{
+ struct drm_exynos_ipp_send_event *e;
+ unsigned long flags;
+
+ DRM_DEBUG_KMS("ops_id[%d]buf_id[%d]\n", qbuf->ops_id, qbuf->buf_id);
+
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
+ if (!e) {
+ spin_lock_irqsave(&drm_dev->event_lock, flags);
+ file->event_space += sizeof(e->event);
+ spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+ return -ENOMEM;
+ }
+
+ /* make event */
+ e->event.base.type = DRM_EXYNOS_IPP_EVENT;
+ e->event.base.length = sizeof(e->event);
+ e->event.user_data = qbuf->user_data;
+ e->event.prop_id = qbuf->prop_id;
+ e->event.buf_id[EXYNOS_DRM_OPS_DST] = qbuf->buf_id;
+ 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;
+}
+
+static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_queue_buf *qbuf)
+{
+ 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);
+
+ /*
+ * qbuf == NULL condition means all event deletion.
+ * stop operations want to delete all event list.
+ * another case delete only same buf id.
+ */
+ if (!qbuf) {
+ /* delete list */
+ list_del(&e->base.link);
+ kfree(e);
+ }
+
+ /* compare buffer id */
+ if (qbuf && (qbuf->buf_id ==
+ e->event.buf_id[EXYNOS_DRM_OPS_DST])) {
+ /* delete list */
+ list_del(&e->base.link);
+ kfree(e);
+ goto out_unlock;
+ }
+ }
+
+out_unlock:
+ mutex_unlock(&c_node->event_lock);
+ return;
+}
+
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
@@ -611,6 +686,9 @@ static void ipp_clean_cmd_node(struct ipp_context *ctx,
cancel_work_sync(&c_node->stop_work->work);
cancel_work_sync(&c_node->event_work->work);
+ /* put event */
+ ipp_put_event(c_node, NULL);
+
for_each_ipp_ops(i)
ipp_clean_mem_nodes(ctx->subdrv.drm_dev, c_node, i);
@@ -707,81 +785,6 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
return ret;
}
-static void ipp_free_event(struct drm_pending_event *event)
-{
- kfree(event);
-}
-
-static int ipp_get_event(struct drm_device *drm_dev,
- struct drm_file *file,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_queue_buf *qbuf)
-{
- struct drm_exynos_ipp_send_event *e;
- unsigned long flags;
-
- DRM_DEBUG_KMS("ops_id[%d]buf_id[%d]\n", qbuf->ops_id, qbuf->buf_id);
-
- e = kzalloc(sizeof(*e), GFP_KERNEL);
- if (!e) {
- spin_lock_irqsave(&drm_dev->event_lock, flags);
- file->event_space += sizeof(e->event);
- spin_unlock_irqrestore(&drm_dev->event_lock, flags);
- return -ENOMEM;
- }
-
- /* make event */
- e->event.base.type = DRM_EXYNOS_IPP_EVENT;
- e->event.base.length = sizeof(e->event);
- e->event.user_data = qbuf->user_data;
- e->event.prop_id = qbuf->prop_id;
- e->event.buf_id[EXYNOS_DRM_OPS_DST] = qbuf->buf_id;
- 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;
-}
-
-static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_queue_buf *qbuf)
-{
- 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);
-
- /*
- * qbuf == NULL condition means all event deletion.
- * stop operations want to delete all event list.
- * another case delete only same buf id.
- */
- if (!qbuf) {
- /* delete list */
- list_del(&e->base.link);
- kfree(e);
- }
-
- /* compare buffer id */
- if (qbuf && (qbuf->buf_id ==
- e->event.buf_id[EXYNOS_DRM_OPS_DST])) {
- /* delete list */
- list_del(&e->base.link);
- kfree(e);
- goto out_unlock;
- }
- }
-
-out_unlock:
- mutex_unlock(&c_node->event_lock);
- return;
-}
-
static void ipp_handle_cmd_work(struct device *dev,
struct exynos_drm_ippdrv *ippdrv,
struct drm_exynos_ipp_cmd_work *cmd_work,
@@ -1287,8 +1290,6 @@ static int ipp_stop_property(struct drm_device *drm_dev,
DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
- /* put event */
- ipp_put_event(c_node, NULL);
/* stop operations */
if (ippdrv->stop)
ippdrv->stop(ippdrv->dev, property->cmd);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 11/15] drm/exynos/ipp: remove events during command cleaning
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
Events were removed only during stop command, as a result
there were memory leaks if program prematurely exited.
This patch fixes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 155 ++++++++++++++++----------------
1 file changed, 78 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 6de75aa..ec88393 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -601,6 +601,81 @@ static void ipp_clean_mem_nodes(struct drm_device *drm_dev,
mutex_unlock(&c_node->mem_lock);
}
+static void ipp_free_event(struct drm_pending_event *event)
+{
+ kfree(event);
+}
+
+static int ipp_get_event(struct drm_device *drm_dev,
+ struct drm_file *file,
+ struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_queue_buf *qbuf)
+{
+ struct drm_exynos_ipp_send_event *e;
+ unsigned long flags;
+
+ DRM_DEBUG_KMS("ops_id[%d]buf_id[%d]\n", qbuf->ops_id, qbuf->buf_id);
+
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
+ if (!e) {
+ spin_lock_irqsave(&drm_dev->event_lock, flags);
+ file->event_space += sizeof(e->event);
+ spin_unlock_irqrestore(&drm_dev->event_lock, flags);
+ return -ENOMEM;
+ }
+
+ /* make event */
+ e->event.base.type = DRM_EXYNOS_IPP_EVENT;
+ e->event.base.length = sizeof(e->event);
+ e->event.user_data = qbuf->user_data;
+ e->event.prop_id = qbuf->prop_id;
+ e->event.buf_id[EXYNOS_DRM_OPS_DST] = qbuf->buf_id;
+ 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;
+}
+
+static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node,
+ struct drm_exynos_ipp_queue_buf *qbuf)
+{
+ 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);
+
+ /*
+ * qbuf == NULL condition means all event deletion.
+ * stop operations want to delete all event list.
+ * another case delete only same buf id.
+ */
+ if (!qbuf) {
+ /* delete list */
+ list_del(&e->base.link);
+ kfree(e);
+ }
+
+ /* compare buffer id */
+ if (qbuf && (qbuf->buf_id ==
+ e->event.buf_id[EXYNOS_DRM_OPS_DST])) {
+ /* delete list */
+ list_del(&e->base.link);
+ kfree(e);
+ goto out_unlock;
+ }
+ }
+
+out_unlock:
+ mutex_unlock(&c_node->event_lock);
+ return;
+}
+
static void ipp_clean_cmd_node(struct ipp_context *ctx,
struct drm_exynos_ipp_cmd_node *c_node)
{
@@ -611,6 +686,9 @@ static void ipp_clean_cmd_node(struct ipp_context *ctx,
cancel_work_sync(&c_node->stop_work->work);
cancel_work_sync(&c_node->event_work->work);
+ /* put event */
+ ipp_put_event(c_node, NULL);
+
for_each_ipp_ops(i)
ipp_clean_mem_nodes(ctx->subdrv.drm_dev, c_node, i);
@@ -707,81 +785,6 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
return ret;
}
-static void ipp_free_event(struct drm_pending_event *event)
-{
- kfree(event);
-}
-
-static int ipp_get_event(struct drm_device *drm_dev,
- struct drm_file *file,
- struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_queue_buf *qbuf)
-{
- struct drm_exynos_ipp_send_event *e;
- unsigned long flags;
-
- DRM_DEBUG_KMS("ops_id[%d]buf_id[%d]\n", qbuf->ops_id, qbuf->buf_id);
-
- e = kzalloc(sizeof(*e), GFP_KERNEL);
- if (!e) {
- spin_lock_irqsave(&drm_dev->event_lock, flags);
- file->event_space += sizeof(e->event);
- spin_unlock_irqrestore(&drm_dev->event_lock, flags);
- return -ENOMEM;
- }
-
- /* make event */
- e->event.base.type = DRM_EXYNOS_IPP_EVENT;
- e->event.base.length = sizeof(e->event);
- e->event.user_data = qbuf->user_data;
- e->event.prop_id = qbuf->prop_id;
- e->event.buf_id[EXYNOS_DRM_OPS_DST] = qbuf->buf_id;
- 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;
-}
-
-static void ipp_put_event(struct drm_exynos_ipp_cmd_node *c_node,
- struct drm_exynos_ipp_queue_buf *qbuf)
-{
- 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);
-
- /*
- * qbuf == NULL condition means all event deletion.
- * stop operations want to delete all event list.
- * another case delete only same buf id.
- */
- if (!qbuf) {
- /* delete list */
- list_del(&e->base.link);
- kfree(e);
- }
-
- /* compare buffer id */
- if (qbuf && (qbuf->buf_id ==
- e->event.buf_id[EXYNOS_DRM_OPS_DST])) {
- /* delete list */
- list_del(&e->base.link);
- kfree(e);
- goto out_unlock;
- }
- }
-
-out_unlock:
- mutex_unlock(&c_node->event_lock);
- return;
-}
-
static void ipp_handle_cmd_work(struct device *dev,
struct exynos_drm_ippdrv *ippdrv,
struct drm_exynos_ipp_cmd_work *cmd_work,
@@ -1287,8 +1290,6 @@ static int ipp_stop_property(struct drm_device *drm_dev,
DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
- /* put event */
- ipp_put_event(c_node, NULL);
/* stop operations */
if (ippdrv->stop)
ippdrv->stop(ippdrv->dev, property->cmd);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 12/15] drm/exynos/fimc: avoid clearing overflow bits
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
Overflow bits shall be cleared by H/W.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 3264ed3..bbaf4f9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -341,9 +341,6 @@ static bool fimc_check_ovf(struct fimc_context *ctx)
fimc_set_bits(ctx, EXYNOS_CIWDOFST,
EXYNOS_CIWDOFST_CLROVFIY | EXYNOS_CIWDOFST_CLROVFICB |
EXYNOS_CIWDOFST_CLROVFICR);
- fimc_clear_bits(ctx, EXYNOS_CIWDOFST,
- EXYNOS_CIWDOFST_CLROVFIY | EXYNOS_CIWDOFST_CLROVFICB |
- EXYNOS_CIWDOFST_CLROVFICR);
dev_err(ippdrv->dev, "occurred overflow at %d, status 0x%x.\n",
ctx->id, status);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 12/15] drm/exynos/fimc: avoid clearing overflow bits
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
Overflow bits shall be cleared by H/W.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 3264ed3..bbaf4f9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -341,9 +341,6 @@ static bool fimc_check_ovf(struct fimc_context *ctx)
fimc_set_bits(ctx, EXYNOS_CIWDOFST,
EXYNOS_CIWDOFST_CLROVFIY | EXYNOS_CIWDOFST_CLROVFICB |
EXYNOS_CIWDOFST_CLROVFICR);
- fimc_clear_bits(ctx, EXYNOS_CIWDOFST,
- EXYNOS_CIWDOFST_CLROVFIY | EXYNOS_CIWDOFST_CLROVFICB |
- EXYNOS_CIWDOFST_CLROVFICR);
dev_err(ippdrv->dev, "occurred overflow at %d, status 0x%x.\n",
ctx->id, status);
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 13/15] drm/exynos/fimc: do not enable fimc twice
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
The patch removes redundant H/W activation.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index bbaf4f9..bd6628d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1596,12 +1596,9 @@ static int fimc_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd)
fimc_clear_bits(ctx, EXYNOS_CIOCTRL, EXYNOS_CIOCTRL_WEAVE_MASK);
- if (cmd == IPP_CMD_M2M) {
+ if (cmd == IPP_CMD_M2M)
fimc_set_bits(ctx, EXYNOS_MSCTRL, EXYNOS_MSCTRL_ENVID);
- fimc_set_bits(ctx, EXYNOS_MSCTRL, EXYNOS_MSCTRL_ENVID);
- }
-
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 13/15] drm/exynos/fimc: do not enable fimc twice
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
The patch removes redundant H/W activation.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index bbaf4f9..bd6628d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1596,12 +1596,9 @@ static int fimc_ippdrv_start(struct device *dev, enum drm_exynos_ipp_cmd cmd)
fimc_clear_bits(ctx, EXYNOS_CIOCTRL, EXYNOS_CIOCTRL_WEAVE_MASK);
- if (cmd == IPP_CMD_M2M) {
+ if (cmd == IPP_CMD_M2M)
fimc_set_bits(ctx, EXYNOS_MSCTRL, EXYNOS_MSCTRL_ENVID);
- fimc_set_bits(ctx, EXYNOS_MSCTRL, EXYNOS_MSCTRL_ENVID);
- }
-
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
The patch removes redundant checks, redundant HW reads
and simplifies code.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
1 file changed, 15 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index bd6628d..b20078e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
return 0;
}
-static int fimc_dst_get_buf_count(struct fimc_context *ctx)
-{
- u32 cfg, buf_num;
-
- cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
-
- buf_num = hweight32(cfg);
-
- DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
-
- return buf_num;
-}
-
-static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
+static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
enum drm_exynos_ipp_buf_type buf_type)
{
- struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
- bool enable;
- u32 cfg;
- u32 mask = 0x00000001 << buf_id;
- int ret = 0;
unsigned long flags;
+ u32 buf_num;
+ u32 cfg;
DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
spin_lock_irqsave(&ctx->lock, flags);
- /* mask register set */
cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
- switch (buf_type) {
- case IPP_BUF_ENQUEUE:
- enable = true;
- break;
- case IPP_BUF_DEQUEUE:
- enable = false;
- break;
- default:
- dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
- ret = -EINVAL;
- goto err_unlock;
- }
+ if (buf_type == IPP_BUF_ENQUEUE)
+ cfg |= (1 << buf_id);
+ else
+ cfg &= (1 << buf_id);
- /* sequence id */
- cfg &= ~mask;
- cfg |= (enable << buf_id);
fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
- /* interrupt enable */
- if (buf_type == IPP_BUF_ENQUEUE &&
- fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
- fimc_mask_irq(ctx, true);
+ buf_num = hweight32(cfg);
- /* interrupt disable */
- if (buf_type == IPP_BUF_DEQUEUE &&
- fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
+ if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
+ fimc_mask_irq(ctx, true);
+ else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
fimc_mask_irq(ctx, false);
-err_unlock:
spin_unlock_irqrestore(&ctx->lock, flags);
- return ret;
}
static int fimc_dst_set_addr(struct device *dev,
@@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev,
break;
}
- return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+ fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+
+ return 0;
}
static struct exynos_drm_ipp_ops fimc_dst_ops = {
@@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
- if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
- DRM_ERROR("failed to dequeue.\n");
- return IRQ_HANDLED;
- }
+ fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
The patch removes redundant checks, redundant HW reads
and simplifies code.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
1 file changed, 15 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index bd6628d..b20078e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
return 0;
}
-static int fimc_dst_get_buf_count(struct fimc_context *ctx)
-{
- u32 cfg, buf_num;
-
- cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
-
- buf_num = hweight32(cfg);
-
- DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
-
- return buf_num;
-}
-
-static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
+static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
enum drm_exynos_ipp_buf_type buf_type)
{
- struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
- bool enable;
- u32 cfg;
- u32 mask = 0x00000001 << buf_id;
- int ret = 0;
unsigned long flags;
+ u32 buf_num;
+ u32 cfg;
DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
spin_lock_irqsave(&ctx->lock, flags);
- /* mask register set */
cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
- switch (buf_type) {
- case IPP_BUF_ENQUEUE:
- enable = true;
- break;
- case IPP_BUF_DEQUEUE:
- enable = false;
- break;
- default:
- dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
- ret = -EINVAL;
- goto err_unlock;
- }
+ if (buf_type == IPP_BUF_ENQUEUE)
+ cfg |= (1 << buf_id);
+ else
+ cfg &= (1 << buf_id);
- /* sequence id */
- cfg &= ~mask;
- cfg |= (enable << buf_id);
fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
- /* interrupt enable */
- if (buf_type == IPP_BUF_ENQUEUE &&
- fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
- fimc_mask_irq(ctx, true);
+ buf_num = hweight32(cfg);
- /* interrupt disable */
- if (buf_type == IPP_BUF_DEQUEUE &&
- fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
+ if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
+ fimc_mask_irq(ctx, true);
+ else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
fimc_mask_irq(ctx, false);
-err_unlock:
spin_unlock_irqrestore(&ctx->lock, flags);
- return ret;
}
static int fimc_dst_set_addr(struct device *dev,
@@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev,
break;
}
- return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+ fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+
+ return 0;
}
static struct exynos_drm_ipp_ops fimc_dst_ops = {
@@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
- if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
- DRM_ERROR("failed to dequeue.\n");
- return IRQ_HANDLED;
- }
+ fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-22 7:52 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
FIMC in default mode of operation uses only one input buffer,
but the driver used also second buffer, as a result only the
first frame was processed correctly. The patch fixes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index b20078e..e985253 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
case IPP_BUF_ENQUEUE:
config = &property->config[EXYNOS_DRM_OPS_SRC];
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
- EXYNOS_CIIYSA(buf_id));
+ EXYNOS_CIIYSA0);
if (config->fmt == DRM_FORMAT_YVU420) {
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
- EXYNOS_CIICBSA(buf_id));
+ EXYNOS_CIICBSA0);
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
- EXYNOS_CIICRSA(buf_id));
+ EXYNOS_CIICRSA0);
} else {
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
- EXYNOS_CIICBSA(buf_id));
+ EXYNOS_CIICBSA0);
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
- EXYNOS_CIICRSA(buf_id));
+ EXYNOS_CIICRSA0);
}
break;
case IPP_BUF_DEQUEUE:
- fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
- fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
- fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
+ fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
+ fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
+ fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
break;
default:
/* bypass */
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
@ 2014-08-22 7:52 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-22 7:52 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
FIMC in default mode of operation uses only one input buffer,
but the driver used also second buffer, as a result only the
first frame was processed correctly. The patch fixes it.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index b20078e..e985253 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
case IPP_BUF_ENQUEUE:
config = &property->config[EXYNOS_DRM_OPS_SRC];
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
- EXYNOS_CIIYSA(buf_id));
+ EXYNOS_CIIYSA0);
if (config->fmt == DRM_FORMAT_YVU420) {
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
- EXYNOS_CIICBSA(buf_id));
+ EXYNOS_CIICBSA0);
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
- EXYNOS_CIICRSA(buf_id));
+ EXYNOS_CIICRSA0);
} else {
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
- EXYNOS_CIICBSA(buf_id));
+ EXYNOS_CIICBSA0);
fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
- EXYNOS_CIICRSA(buf_id));
+ EXYNOS_CIICRSA0);
}
break;
case IPP_BUF_DEQUEUE:
- fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
- fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
- fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
+ fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
+ fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
+ fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
break;
default:
/* bypass */
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-26 2:55 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 2:55 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> Command node should contain file reference to distinguish commands
> created by different processes.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 9770966..bbe9968 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
> u32 prop_id;
> u32 buf_id;
> struct drm_exynos_ipp_buf_info buf_info;
> - struct drm_file *filp;
> };
>
> /*
> @@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
> c_node->dev = dev;
> c_node->property = *property;
> c_node->state = IPP_STATE_IDLE;
> + c_node->filp = file;
>
> c_node->start_work = ipp_create_cmd_work();
> if (IS_ERR(c_node->start_work)) {
> @@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
> }
> }
>
> - m_node->filp = file;
> 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);
> @@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
Then, could you remove file argument from exynos_drm_ipp_queue_buf() and
ipp_get_event()?
> unsigned long handle = m_node->buf_info.handles[i];
> if (handle)
> exynos_drm_gem_put_dma_addr(drm_dev, handle,
> - m_node->filp);
> + c_node->filp);
> }
>
> /* delete list in queue */
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> index 6f48d62..0311035 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> @@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
> * @stop_work: stop command work structure.
> * @event_work: event work structure.
> * @state: state of command node.
> + * @filp: associated file pointer.
> */
> struct drm_exynos_ipp_cmd_node {
> struct device *dev;
> @@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
> struct drm_exynos_ipp_cmd_work *stop_work;
> struct drm_exynos_ipp_event_work *event_work;
> enum drm_exynos_ipp_state state;
> + struct drm_file *filp;
> };
>
> /*
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
@ 2014-08-26 2:55 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 2:55 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Kyungmin Park,
Marek Szyprowski
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> Command node should contain file reference to distinguish commands
> created by different processes.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 9770966..bbe9968 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
> u32 prop_id;
> u32 buf_id;
> struct drm_exynos_ipp_buf_info buf_info;
> - struct drm_file *filp;
> };
>
> /*
> @@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
> c_node->dev = dev;
> c_node->property = *property;
> c_node->state = IPP_STATE_IDLE;
> + c_node->filp = file;
>
> c_node->start_work = ipp_create_cmd_work();
> if (IS_ERR(c_node->start_work)) {
> @@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
> }
> }
>
> - m_node->filp = file;
> 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);
> @@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
Then, could you remove file argument from exynos_drm_ipp_queue_buf() and
ipp_get_event()?
> unsigned long handle = m_node->buf_info.handles[i];
> if (handle)
> exynos_drm_gem_put_dma_addr(drm_dev, handle,
> - m_node->filp);
> + c_node->filp);
> }
>
> /* delete list in queue */
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> index 6f48d62..0311035 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
> @@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
> * @stop_work: stop command work structure.
> * @event_work: event work structure.
> * @state: state of command node.
> + * @filp: associated file pointer.
> */
> struct drm_exynos_ipp_cmd_node {
> struct device *dev;
> @@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
> struct drm_exynos_ipp_cmd_work *stop_work;
> struct drm_exynos_ipp_event_work *event_work;
> enum drm_exynos_ipp_state state;
> + struct drm_file *filp;
> };
>
> /*
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
2014-08-26 2:55 ` Joonyoung Shim
@ 2014-08-26 2:59 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 2:59 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
On 08/26/2014 11:55 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> Command node should contain file reference to distinguish commands
>> created by different processes.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
>> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> index 9770966..bbe9968 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> @@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
>> u32 prop_id;
>> u32 buf_id;
>> struct drm_exynos_ipp_buf_info buf_info;
>> - struct drm_file *filp;
>> };
>>
>> /*
>> @@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
>> c_node->dev = dev;
>> c_node->property = *property;
>> c_node->state = IPP_STATE_IDLE;
>> + c_node->filp = file;
>>
>> c_node->start_work = ipp_create_cmd_work();
>> if (IS_ERR(c_node->start_work)) {
>> @@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
>> }
>> }
>>
>> - m_node->filp = file;
>> 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);
>> @@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
>
> Then, could you remove file argument from exynos_drm_ipp_queue_buf() and
> ipp_get_event()?
sorry, i mean ipp_put_mem_node() instead of exynos_drm_ipp_queue_buf().
>
>> unsigned long handle = m_node->buf_info.handles[i];
>> if (handle)
>> exynos_drm_gem_put_dma_addr(drm_dev, handle,
>> - m_node->filp);
>> + c_node->filp);
>> }
>>
>> /* delete list in queue */
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>> index 6f48d62..0311035 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>> @@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
>> * @stop_work: stop command work structure.
>> * @event_work: event work structure.
>> * @state: state of command node.
>> + * @filp: associated file pointer.
>> */
>> struct drm_exynos_ipp_cmd_node {
>> struct device *dev;
>> @@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
>> struct drm_exynos_ipp_cmd_work *stop_work;
>> struct drm_exynos_ipp_event_work *event_work;
>> enum drm_exynos_ipp_state state;
>> + struct drm_file *filp;
>> };
>>
>> /*
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
@ 2014-08-26 2:59 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 2:59 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
On 08/26/2014 11:55 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> Command node should contain file reference to distinguish commands
>> created by different processes.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
>> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> index 9770966..bbe9968 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> @@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
>> u32 prop_id;
>> u32 buf_id;
>> struct drm_exynos_ipp_buf_info buf_info;
>> - struct drm_file *filp;
>> };
>>
>> /*
>> @@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
>> c_node->dev = dev;
>> c_node->property = *property;
>> c_node->state = IPP_STATE_IDLE;
>> + c_node->filp = file;
>>
>> c_node->start_work = ipp_create_cmd_work();
>> if (IS_ERR(c_node->start_work)) {
>> @@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
>> }
>> }
>>
>> - m_node->filp = file;
>> 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);
>> @@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
>
> Then, could you remove file argument from exynos_drm_ipp_queue_buf() and
> ipp_get_event()?
sorry, i mean ipp_put_mem_node() instead of exynos_drm_ipp_queue_buf().
>
>> unsigned long handle = m_node->buf_info.handles[i];
>> if (handle)
>> exynos_drm_gem_put_dma_addr(drm_dev, handle,
>> - m_node->filp);
>> + c_node->filp);
>> }
>>
>> /* delete list in queue */
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>> index 6f48d62..0311035 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>> @@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
>> * @stop_work: stop command work structure.
>> * @event_work: event work structure.
>> * @state: state of command node.
>> + * @filp: associated file pointer.
>> */
>> struct drm_exynos_ipp_cmd_node {
>> struct device *dev;
>> @@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
>> struct drm_exynos_ipp_cmd_work *stop_work;
>> struct drm_exynos_ipp_event_work *event_work;
>> enum drm_exynos_ipp_state state;
>> + struct drm_file *filp;
>> };
>>
>> /*
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-26 5:00 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 5:00 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> In case of allocation errors some already allocated buffers
> were not freed. The patch fixes it.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 060a198..728f3b9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
> return ret;
> }
>
> +static int ipp_put_mem_node(struct drm_device *drm_dev,
> + struct drm_exynos_ipp_cmd_node *c_node,
> + struct drm_exynos_ipp_mem_node *m_node)
> +{
> + int i;
> +
> + DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
> +
> + if (!m_node) {
> + DRM_ERROR("invalid dequeue node.\n");
> + return -EFAULT;
> + }
> +
> + DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
> +
> + /* put gem buffer */
> + for_each_ipp_planar(i) {
> + unsigned long handle = m_node->buf_info.handles[i];
> + if (handle)
> + exynos_drm_gem_put_dma_addr(drm_dev, handle,
> + c_node->filp);
> + }
> +
> + /* conditionally remove from queue */
> + if (m_node->list.next)
How about do INIT_LIST_HEAD for list in ipp_get_mem_node()?
> + list_del(&m_node->list);
> + kfree(m_node);
> +
> + return 0;
> +}
> +
> static struct drm_exynos_ipp_mem_node
> *ipp_get_mem_node(struct drm_device *drm_dev,
> struct drm_file *file,
> @@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
> qbuf->handle[i], file);
> if (IS_ERR(addr)) {
> DRM_ERROR("failed to get addr.\n");
> - goto err_clear;
> + ipp_put_mem_node(drm_dev, c_node, m_node);
> + return ERR_PTR(-EFAULT);
> }
>
> buf_info->handles[i] = qbuf->handle[i];
> @@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
> mutex_unlock(&c_node->mem_lock);
>
> return m_node;
> -
> -err_clear:
> - kfree(m_node);
> - return ERR_PTR(-EFAULT);
> -}
> -
> -static int ipp_put_mem_node(struct drm_device *drm_dev,
> - struct drm_exynos_ipp_cmd_node *c_node,
> - struct drm_exynos_ipp_mem_node *m_node)
> -{
> - int i;
> -
> - DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
> -
> - if (!m_node) {
> - DRM_ERROR("invalid dequeue node.\n");
> - return -EFAULT;
> - }
> -
> - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
> -
> - /* put gem buffer */
> - for_each_ipp_planar(i) {
> - unsigned long handle = m_node->buf_info.handles[i];
> - if (handle)
> - exynos_drm_gem_put_dma_addr(drm_dev, handle,
> - c_node->filp);
> - }
> -
> - /* delete list in queue */
> - list_del(&m_node->list);
> - kfree(m_node);
> -
> - return 0;
> }
>
> static void ipp_free_event(struct drm_pending_event *event)
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
@ 2014-08-26 5:00 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 5:00 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> In case of allocation errors some already allocated buffers
> were not freed. The patch fixes it.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 060a198..728f3b9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
> return ret;
> }
>
> +static int ipp_put_mem_node(struct drm_device *drm_dev,
> + struct drm_exynos_ipp_cmd_node *c_node,
> + struct drm_exynos_ipp_mem_node *m_node)
> +{
> + int i;
> +
> + DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
> +
> + if (!m_node) {
> + DRM_ERROR("invalid dequeue node.\n");
> + return -EFAULT;
> + }
> +
> + DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
> +
> + /* put gem buffer */
> + for_each_ipp_planar(i) {
> + unsigned long handle = m_node->buf_info.handles[i];
> + if (handle)
> + exynos_drm_gem_put_dma_addr(drm_dev, handle,
> + c_node->filp);
> + }
> +
> + /* conditionally remove from queue */
> + if (m_node->list.next)
How about do INIT_LIST_HEAD for list in ipp_get_mem_node()?
> + list_del(&m_node->list);
> + kfree(m_node);
> +
> + return 0;
> +}
> +
> static struct drm_exynos_ipp_mem_node
> *ipp_get_mem_node(struct drm_device *drm_dev,
> struct drm_file *file,
> @@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
> qbuf->handle[i], file);
> if (IS_ERR(addr)) {
> DRM_ERROR("failed to get addr.\n");
> - goto err_clear;
> + ipp_put_mem_node(drm_dev, c_node, m_node);
> + return ERR_PTR(-EFAULT);
> }
>
> buf_info->handles[i] = qbuf->handle[i];
> @@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
> mutex_unlock(&c_node->mem_lock);
>
> return m_node;
> -
> -err_clear:
> - kfree(m_node);
> - return ERR_PTR(-EFAULT);
> -}
> -
> -static int ipp_put_mem_node(struct drm_device *drm_dev,
> - struct drm_exynos_ipp_cmd_node *c_node,
> - struct drm_exynos_ipp_mem_node *m_node)
> -{
> - int i;
> -
> - DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
> -
> - if (!m_node) {
> - DRM_ERROR("invalid dequeue node.\n");
> - return -EFAULT;
> - }
> -
> - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
> -
> - /* put gem buffer */
> - for_each_ipp_planar(i) {
> - unsigned long handle = m_node->buf_info.handles[i];
> - if (handle)
> - exynos_drm_gem_put_dma_addr(drm_dev, handle,
> - c_node->filp);
> - }
> -
> - /* delete list in queue */
> - list_del(&m_node->list);
> - kfree(m_node);
> -
> - return 0;
> }
>
> static void ipp_free_event(struct drm_pending_event *event)
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-26 5:53 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 5:53 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> The patch removes redundant checks, redundant HW reads
> and simplifies code.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
> 1 file changed, 15 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index bd6628d..b20078e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
> return 0;
> }
>
> -static int fimc_dst_get_buf_count(struct fimc_context *ctx)
> -{
> - u32 cfg, buf_num;
> -
> - cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
> -
> - buf_num = hweight32(cfg);
> -
> - DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
> -
> - return buf_num;
> -}
> -
> -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
> +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
> enum drm_exynos_ipp_buf_type buf_type)
> {
> - struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
> - bool enable;
> - u32 cfg;
> - u32 mask = 0x00000001 << buf_id;
> - int ret = 0;
> unsigned long flags;
> + u32 buf_num;
> + u32 cfg;
>
> DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
>
> spin_lock_irqsave(&ctx->lock, flags);
>
> - /* mask register set */
> cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
>
> - switch (buf_type) {
> - case IPP_BUF_ENQUEUE:
> - enable = true;
> - break;
> - case IPP_BUF_DEQUEUE:
> - enable = false;
> - break;
> - default:
> - dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
> - ret = -EINVAL;
> - goto err_unlock;
> - }
> + if (buf_type == IPP_BUF_ENQUEUE)
> + cfg |= (1 << buf_id);
> + else
> + cfg &= (1 << buf_id);
~ Missing?
>
> - /* sequence id */
> - cfg &= ~mask;
> - cfg |= (enable << buf_id);
> fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
>
> - /* interrupt enable */
> - if (buf_type == IPP_BUF_ENQUEUE &&
> - fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
> - fimc_mask_irq(ctx, true);
> + buf_num = hweight32(cfg);
>
> - /* interrupt disable */
> - if (buf_type == IPP_BUF_DEQUEUE &&
> - fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
> + if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
> + fimc_mask_irq(ctx, true);
> + else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
> fimc_mask_irq(ctx, false);
>
> -err_unlock:
> spin_unlock_irqrestore(&ctx->lock, flags);
> - return ret;
> }
>
> static int fimc_dst_set_addr(struct device *dev,
> @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev,
> break;
> }
>
> - return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
> + fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
> +
> + return 0;
> }
>
> static struct exynos_drm_ipp_ops fimc_dst_ops = {
> @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
>
> DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
>
> - if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
> - DRM_ERROR("failed to dequeue.\n");
> - return IRQ_HANDLED;
> - }
> + fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
>
> event_work->ippdrv = ippdrv;
> event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
@ 2014-08-26 5:53 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 5:53 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> The patch removes redundant checks, redundant HW reads
> and simplifies code.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
> 1 file changed, 15 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index bd6628d..b20078e 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
> return 0;
> }
>
> -static int fimc_dst_get_buf_count(struct fimc_context *ctx)
> -{
> - u32 cfg, buf_num;
> -
> - cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
> -
> - buf_num = hweight32(cfg);
> -
> - DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
> -
> - return buf_num;
> -}
> -
> -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
> +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
> enum drm_exynos_ipp_buf_type buf_type)
> {
> - struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
> - bool enable;
> - u32 cfg;
> - u32 mask = 0x00000001 << buf_id;
> - int ret = 0;
> unsigned long flags;
> + u32 buf_num;
> + u32 cfg;
>
> DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
>
> spin_lock_irqsave(&ctx->lock, flags);
>
> - /* mask register set */
> cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
>
> - switch (buf_type) {
> - case IPP_BUF_ENQUEUE:
> - enable = true;
> - break;
> - case IPP_BUF_DEQUEUE:
> - enable = false;
> - break;
> - default:
> - dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
> - ret = -EINVAL;
> - goto err_unlock;
> - }
> + if (buf_type == IPP_BUF_ENQUEUE)
> + cfg |= (1 << buf_id);
> + else
> + cfg &= (1 << buf_id);
~ Missing?
>
> - /* sequence id */
> - cfg &= ~mask;
> - cfg |= (enable << buf_id);
> fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
>
> - /* interrupt enable */
> - if (buf_type == IPP_BUF_ENQUEUE &&
> - fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
> - fimc_mask_irq(ctx, true);
> + buf_num = hweight32(cfg);
>
> - /* interrupt disable */
> - if (buf_type == IPP_BUF_DEQUEUE &&
> - fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
> + if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
> + fimc_mask_irq(ctx, true);
> + else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
> fimc_mask_irq(ctx, false);
>
> -err_unlock:
> spin_unlock_irqrestore(&ctx->lock, flags);
> - return ret;
> }
>
> static int fimc_dst_set_addr(struct device *dev,
> @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev,
> break;
> }
>
> - return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
> + fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
> +
> + return 0;
> }
>
> static struct exynos_drm_ipp_ops fimc_dst_ops = {
> @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
>
> DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
>
> - if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
> - DRM_ERROR("failed to dequeue.\n");
> - return IRQ_HANDLED;
> - }
> + fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
>
> event_work->ippdrv = ippdrv;
> event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-26 5:57 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 5:57 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> FIMC in default mode of operation uses only one input buffer,
> but the driver used also second buffer, as a result only the
> first frame was processed correctly. The patch fixes it.
I can't understand well, then we don't need to distinguish buf_id in
fimc_src_set_addr()?
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index b20078e..e985253 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
> case IPP_BUF_ENQUEUE:
> config = &property->config[EXYNOS_DRM_OPS_SRC];
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
> - EXYNOS_CIIYSA(buf_id));
> + EXYNOS_CIIYSA0);
>
> if (config->fmt == DRM_FORMAT_YVU420) {
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
> - EXYNOS_CIICBSA(buf_id));
> + EXYNOS_CIICBSA0);
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
> - EXYNOS_CIICRSA(buf_id));
> + EXYNOS_CIICRSA0);
> } else {
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
> - EXYNOS_CIICBSA(buf_id));
> + EXYNOS_CIICBSA0);
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
> - EXYNOS_CIICRSA(buf_id));
> + EXYNOS_CIICRSA0);
> }
> break;
> case IPP_BUF_DEQUEUE:
> - fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
> - fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
> - fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
> + fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
> + fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
> + fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
> break;
> default:
> /* bypass */
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
@ 2014-08-26 5:57 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 5:57 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi Andrzej,
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> FIMC in default mode of operation uses only one input buffer,
> but the driver used also second buffer, as a result only the
> first frame was processed correctly. The patch fixes it.
I can't understand well, then we don't need to distinguish buf_id in
fimc_src_set_addr()?
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index b20078e..e985253 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
> case IPP_BUF_ENQUEUE:
> config = &property->config[EXYNOS_DRM_OPS_SRC];
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
> - EXYNOS_CIIYSA(buf_id));
> + EXYNOS_CIIYSA0);
>
> if (config->fmt == DRM_FORMAT_YVU420) {
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
> - EXYNOS_CIICBSA(buf_id));
> + EXYNOS_CIICBSA0);
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
> - EXYNOS_CIICRSA(buf_id));
> + EXYNOS_CIICRSA0);
> } else {
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
> - EXYNOS_CIICBSA(buf_id));
> + EXYNOS_CIICBSA0);
> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
> - EXYNOS_CIICRSA(buf_id));
> + EXYNOS_CIICRSA0);
> }
> break;
> case IPP_BUF_DEQUEUE:
> - fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
> - fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
> - fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
> + fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
> + fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
> + fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
> break;
> default:
> /* bypass */
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
2014-08-26 2:59 ` Joonyoung Shim
@ 2014-08-26 6:16 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-26 6:16 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
Hi Joonyoung,
Thanks for review.
On 08/26/2014 04:59 AM, Joonyoung Shim wrote:
> On 08/26/2014 11:55 AM, Joonyoung Shim wrote:
>> Hi Andrzej,
>>
>> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>>> Command node should contain file reference to distinguish commands
>>> created by different processes.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
>>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> index 9770966..bbe9968 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> @@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
>>> u32 prop_id;
>>> u32 buf_id;
>>> struct drm_exynos_ipp_buf_info buf_info;
>>> - struct drm_file *filp;
>>> };
>>>
>>> /*
>>> @@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
>>> c_node->dev = dev;
>>> c_node->property = *property;
>>> c_node->state = IPP_STATE_IDLE;
>>> + c_node->filp = file;
>>>
>>> c_node->start_work = ipp_create_cmd_work();
>>> if (IS_ERR(c_node->start_work)) {
>>> @@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
>>> }
>>> }
>>>
>>> - m_node->filp = file;
>>> 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);
>>> @@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
>> Then, could you remove file argument from exynos_drm_ipp_queue_buf() and
>> ipp_get_event()?
> sorry, i mean ipp_put_mem_node() instead of exynos_drm_ipp_queue_buf().
I guess you mean ipp_get_mem_node() and ipp_get_event().
You are right, it should be removed. Additionally file check should be added
to exynos_drm_ipp_queue_buf.
Regards
Andrzej
>
>>> unsigned long handle = m_node->buf_info.handles[i];
>>> if (handle)
>>> exynos_drm_gem_put_dma_addr(drm_dev, handle,
>>> - m_node->filp);
>>> + c_node->filp);
>>> }
>>>
>>> /* delete list in queue */
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>>> index 6f48d62..0311035 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>>> @@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
>>> * @stop_work: stop command work structure.
>>> * @event_work: event work structure.
>>> * @state: state of command node.
>>> + * @filp: associated file pointer.
>>> */
>>> struct drm_exynos_ipp_cmd_node {
>>> struct device *dev;
>>> @@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
>>> struct drm_exynos_ipp_cmd_work *stop_work;
>>> struct drm_exynos_ipp_event_work *event_work;
>>> enum drm_exynos_ipp_state state;
>>> + struct drm_file *filp;
>>> };
>>>
>>> /*
>>>
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node
@ 2014-08-26 6:16 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-26 6:16 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Kyungmin Park,
Marek Szyprowski
Hi Joonyoung,
Thanks for review.
On 08/26/2014 04:59 AM, Joonyoung Shim wrote:
> On 08/26/2014 11:55 AM, Joonyoung Shim wrote:
>> Hi Andrzej,
>>
>> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>>> Command node should contain file reference to distinguish commands
>>> created by different processes.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 ++---
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 2 ++
>>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> index 9770966..bbe9968 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> @@ -75,7 +75,6 @@ struct drm_exynos_ipp_mem_node {
>>> u32 prop_id;
>>> u32 buf_id;
>>> struct drm_exynos_ipp_buf_info buf_info;
>>> - struct drm_file *filp;
>>> };
>>>
>>> /*
>>> @@ -448,6 +447,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
>>> c_node->dev = dev;
>>> c_node->property = *property;
>>> c_node->state = IPP_STATE_IDLE;
>>> + c_node->filp = file;
>>>
>>> c_node->start_work = ipp_create_cmd_work();
>>> if (IS_ERR(c_node->start_work)) {
>>> @@ -645,7 +645,6 @@ static struct drm_exynos_ipp_mem_node
>>> }
>>> }
>>>
>>> - m_node->filp = file;
>>> 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);
>>> @@ -677,7 +676,7 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
>> Then, could you remove file argument from exynos_drm_ipp_queue_buf() and
>> ipp_get_event()?
> sorry, i mean ipp_put_mem_node() instead of exynos_drm_ipp_queue_buf().
I guess you mean ipp_get_mem_node() and ipp_get_event().
You are right, it should be removed. Additionally file check should be added
to exynos_drm_ipp_queue_buf.
Regards
Andrzej
>
>>> unsigned long handle = m_node->buf_info.handles[i];
>>> if (handle)
>>> exynos_drm_gem_put_dma_addr(drm_dev, handle,
>>> - m_node->filp);
>>> + c_node->filp);
>>> }
>>>
>>> /* delete list in queue */
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>>> index 6f48d62..0311035 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
>>> @@ -62,6 +62,7 @@ struct drm_exynos_ipp_cmd_work {
>>> * @stop_work: stop command work structure.
>>> * @event_work: event work structure.
>>> * @state: state of command node.
>>> + * @filp: associated file pointer.
>>> */
>>> struct drm_exynos_ipp_cmd_node {
>>> struct device *dev;
>>> @@ -78,6 +79,7 @@ struct drm_exynos_ipp_cmd_node {
>>> struct drm_exynos_ipp_cmd_work *stop_work;
>>> struct drm_exynos_ipp_event_work *event_work;
>>> enum drm_exynos_ipp_state state;
>>> + struct drm_file *filp;
>>> };
>>>
>>> /*
>>>
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
2014-08-26 5:53 ` Joonyoung Shim
@ 2014-08-26 6:20 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-26 6:20 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
On 08/26/2014 07:53 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> The patch removes redundant checks, redundant HW reads
>> and simplifies code.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
>> 1 file changed, 15 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> index bd6628d..b20078e 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
>> return 0;
>> }
>>
>> -static int fimc_dst_get_buf_count(struct fimc_context *ctx)
>> -{
>> - u32 cfg, buf_num;
>> -
>> - cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
>> -
>> - buf_num = hweight32(cfg);
>> -
>> - DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
>> -
>> - return buf_num;
>> -}
>> -
>> -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
>> +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
>> enum drm_exynos_ipp_buf_type buf_type)
>> {
>> - struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
>> - bool enable;
>> - u32 cfg;
>> - u32 mask = 0x00000001 << buf_id;
>> - int ret = 0;
>> unsigned long flags;
>> + u32 buf_num;
>> + u32 cfg;
>>
>> DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
>>
>> spin_lock_irqsave(&ctx->lock, flags);
>>
>> - /* mask register set */
>> cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
>>
>> - switch (buf_type) {
>> - case IPP_BUF_ENQUEUE:
>> - enable = true;
>> - break;
>> - case IPP_BUF_DEQUEUE:
>> - enable = false;
>> - break;
>> - default:
>> - dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
>> - ret = -EINVAL;
>> - goto err_unlock;
>> - }
>> + if (buf_type == IPP_BUF_ENQUEUE)
>> + cfg |= (1 << buf_id);
>> + else
>> + cfg &= (1 << buf_id);
> ~ Missing?
Yes, thanks for pointing it out.
Regards
Andrzej
>
>>
>> - /* sequence id */
>> - cfg &= ~mask;
>> - cfg |= (enable << buf_id);
>> fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
>>
>> - /* interrupt enable */
>> - if (buf_type == IPP_BUF_ENQUEUE &&
>> - fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
>> - fimc_mask_irq(ctx, true);
>> + buf_num = hweight32(cfg);
>>
>> - /* interrupt disable */
>> - if (buf_type == IPP_BUF_DEQUEUE &&
>> - fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
>> + if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
>> + fimc_mask_irq(ctx, true);
>> + else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
>> fimc_mask_irq(ctx, false);
>>
>> -err_unlock:
>> spin_unlock_irqrestore(&ctx->lock, flags);
>> - return ret;
>> }
>>
>> static int fimc_dst_set_addr(struct device *dev,
>> @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev,
>> break;
>> }
>>
>> - return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
>> + fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
>> +
>> + return 0;
>> }
>>
>> static struct exynos_drm_ipp_ops fimc_dst_ops = {
>> @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
>>
>> DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
>>
>> - if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
>> - DRM_ERROR("failed to dequeue.\n");
>> - return IRQ_HANDLED;
>> - }
>> + fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
>>
>> event_work->ippdrv = ippdrv;
>> event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing
@ 2014-08-26 6:20 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-26 6:20 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Kyungmin Park,
Marek Szyprowski
On 08/26/2014 07:53 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> The patch removes redundant checks, redundant HW reads
>> and simplifies code.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
>> 1 file changed, 15 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> index bd6628d..b20078e 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -1124,67 +1124,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
>> return 0;
>> }
>>
>> -static int fimc_dst_get_buf_count(struct fimc_context *ctx)
>> -{
>> - u32 cfg, buf_num;
>> -
>> - cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
>> -
>> - buf_num = hweight32(cfg);
>> -
>> - DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
>> -
>> - return buf_num;
>> -}
>> -
>> -static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
>> +static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
>> enum drm_exynos_ipp_buf_type buf_type)
>> {
>> - struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
>> - bool enable;
>> - u32 cfg;
>> - u32 mask = 0x00000001 << buf_id;
>> - int ret = 0;
>> unsigned long flags;
>> + u32 buf_num;
>> + u32 cfg;
>>
>> DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
>>
>> spin_lock_irqsave(&ctx->lock, flags);
>>
>> - /* mask register set */
>> cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
>>
>> - switch (buf_type) {
>> - case IPP_BUF_ENQUEUE:
>> - enable = true;
>> - break;
>> - case IPP_BUF_DEQUEUE:
>> - enable = false;
>> - break;
>> - default:
>> - dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
>> - ret = -EINVAL;
>> - goto err_unlock;
>> - }
>> + if (buf_type == IPP_BUF_ENQUEUE)
>> + cfg |= (1 << buf_id);
>> + else
>> + cfg &= (1 << buf_id);
> ~ Missing?
Yes, thanks for pointing it out.
Regards
Andrzej
>
>>
>> - /* sequence id */
>> - cfg &= ~mask;
>> - cfg |= (enable << buf_id);
>> fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
>>
>> - /* interrupt enable */
>> - if (buf_type == IPP_BUF_ENQUEUE &&
>> - fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
>> - fimc_mask_irq(ctx, true);
>> + buf_num = hweight32(cfg);
>>
>> - /* interrupt disable */
>> - if (buf_type == IPP_BUF_DEQUEUE &&
>> - fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
>> + if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
>> + fimc_mask_irq(ctx, true);
>> + else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
>> fimc_mask_irq(ctx, false);
>>
>> -err_unlock:
>> spin_unlock_irqrestore(&ctx->lock, flags);
>> - return ret;
>> }
>>
>> static int fimc_dst_set_addr(struct device *dev,
>> @@ -1242,7 +1209,9 @@ static int fimc_dst_set_addr(struct device *dev,
>> break;
>> }
>>
>> - return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
>> + fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
>> +
>> + return 0;
>> }
>>
>> static struct exynos_drm_ipp_ops fimc_dst_ops = {
>> @@ -1297,10 +1266,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
>>
>> DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
>>
>> - if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
>> - DRM_ERROR("failed to dequeue.\n");
>> - return IRQ_HANDLED;
>> - }
>> + fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
>>
>> event_work->ippdrv = ippdrv;
>> event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
2014-08-26 5:57 ` Joonyoung Shim
@ 2014-08-26 6:35 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-26 6:35 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
On 08/26/2014 07:57 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> FIMC in default mode of operation uses only one input buffer,
>> but the driver used also second buffer, as a result only the
>> first frame was processed correctly. The patch fixes it.
> I can't understand well, then we don't need to distinguish buf_id in
> fimc_src_set_addr()?
Yes. FIMC in default operation mode uses only one input buffer pointer
which should
be updated when processing of the previous buffer has been finished.
There exists also ping-pong mode which uses two buffer pointers, as I
have spotted in
specs. However I have not seen it was implemented neither in drm,
neither in camera drivers.
I will try to implement it later.
Regards
Andrzej
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> index b20078e..e985253 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
>> case IPP_BUF_ENQUEUE:
>> config = &property->config[EXYNOS_DRM_OPS_SRC];
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
>> - EXYNOS_CIIYSA(buf_id));
>> + EXYNOS_CIIYSA0);
>>
>> if (config->fmt == DRM_FORMAT_YVU420) {
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>> - EXYNOS_CIICBSA(buf_id));
>> + EXYNOS_CIICBSA0);
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>> - EXYNOS_CIICRSA(buf_id));
>> + EXYNOS_CIICRSA0);
>> } else {
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>> - EXYNOS_CIICBSA(buf_id));
>> + EXYNOS_CIICBSA0);
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>> - EXYNOS_CIICRSA(buf_id));
>> + EXYNOS_CIICRSA0);
>> }
>> break;
>> case IPP_BUF_DEQUEUE:
>> - fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
>> - fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
>> - fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
>> + fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
>> + fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
>> + fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
>> break;
>> default:
>> /* bypass */
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
@ 2014-08-26 6:35 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-26 6:35 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Kyungmin Park,
Marek Szyprowski
On 08/26/2014 07:57 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> FIMC in default mode of operation uses only one input buffer,
>> but the driver used also second buffer, as a result only the
>> first frame was processed correctly. The patch fixes it.
> I can't understand well, then we don't need to distinguish buf_id in
> fimc_src_set_addr()?
Yes. FIMC in default operation mode uses only one input buffer pointer
which should
be updated when processing of the previous buffer has been finished.
There exists also ping-pong mode which uses two buffer pointers, as I
have spotted in
specs. However I have not seen it was implemented neither in drm,
neither in camera drivers.
I will try to implement it later.
Regards
Andrzej
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> index b20078e..e985253 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>> @@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
>> case IPP_BUF_ENQUEUE:
>> config = &property->config[EXYNOS_DRM_OPS_SRC];
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
>> - EXYNOS_CIIYSA(buf_id));
>> + EXYNOS_CIIYSA0);
>>
>> if (config->fmt == DRM_FORMAT_YVU420) {
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>> - EXYNOS_CIICBSA(buf_id));
>> + EXYNOS_CIICBSA0);
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>> - EXYNOS_CIICRSA(buf_id));
>> + EXYNOS_CIICRSA0);
>> } else {
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>> - EXYNOS_CIICBSA(buf_id));
>> + EXYNOS_CIICBSA0);
>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>> - EXYNOS_CIICRSA(buf_id));
>> + EXYNOS_CIICRSA0);
>> }
>> break;
>> case IPP_BUF_DEQUEUE:
>> - fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
>> - fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
>> - fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
>> + fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
>> + fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
>> + fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
>> break;
>> default:
>> /* bypass */
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
2014-08-26 6:35 ` Andrzej Hajda
@ 2014-08-26 6:47 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 6:47 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
On 08/26/2014 03:35 PM, Andrzej Hajda wrote:
> On 08/26/2014 07:57 AM, Joonyoung Shim wrote:
>> Hi Andrzej,
>>
>> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>>> FIMC in default mode of operation uses only one input buffer,
>>> but the driver used also second buffer, as a result only the
>>> first frame was processed correctly. The patch fixes it.
>> I can't understand well, then we don't need to distinguish buf_id in
>> fimc_src_set_addr()?
> Yes. FIMC in default operation mode uses only one input buffer pointer
> which should
> be updated when processing of the previous buffer has been finished.
>
> There exists also ping-pong mode which uses two buffer pointers, as I
> have spotted in
> specs. However I have not seen it was implemented neither in drm,
> neither in camera drivers.
> I will try to implement it later.
OK if operation is no problem, and i think it's better to add comments
about this.
>
> Regards
> Andrzej
>
>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
>>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> index b20078e..e985253 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> @@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
>>> case IPP_BUF_ENQUEUE:
>>> config = &property->config[EXYNOS_DRM_OPS_SRC];
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
>>> - EXYNOS_CIIYSA(buf_id));
>>> + EXYNOS_CIIYSA0);
>>>
>>> if (config->fmt == DRM_FORMAT_YVU420) {
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>>> - EXYNOS_CIICBSA(buf_id));
>>> + EXYNOS_CIICBSA0);
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>>> - EXYNOS_CIICRSA(buf_id));
>>> + EXYNOS_CIICRSA0);
>>> } else {
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>>> - EXYNOS_CIICBSA(buf_id));
>>> + EXYNOS_CIICBSA0);
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>>> - EXYNOS_CIICRSA(buf_id));
>>> + EXYNOS_CIICRSA0);
>>> }
>>> break;
>>> case IPP_BUF_DEQUEUE:
>>> - fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
>>> - fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
>>> - fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
>>> + fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
>>> + fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
>>> + fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
>>> break;
>>> default:
>>> /* bypass */
>>>
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 15/15] drm/exynos/fimc: fix source buffer registers
@ 2014-08-26 6:47 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 6:47 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
On 08/26/2014 03:35 PM, Andrzej Hajda wrote:
> On 08/26/2014 07:57 AM, Joonyoung Shim wrote:
>> Hi Andrzej,
>>
>> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>>> FIMC in default mode of operation uses only one input buffer,
>>> but the driver used also second buffer, as a result only the
>>> first frame was processed correctly. The patch fixes it.
>> I can't understand well, then we don't need to distinguish buf_id in
>> fimc_src_set_addr()?
> Yes. FIMC in default operation mode uses only one input buffer pointer
> which should
> be updated when processing of the previous buffer has been finished.
>
> There exists also ping-pong mode which uses two buffer pointers, as I
> have spotted in
> specs. However I have not seen it was implemented neither in drm,
> neither in camera drivers.
> I will try to implement it later.
OK if operation is no problem, and i think it's better to add comments
about this.
>
> Regards
> Andrzej
>
>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 16 ++++++++--------
>>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> index b20078e..e985253 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
>>> @@ -720,24 +720,24 @@ static int fimc_src_set_addr(struct device *dev,
>>> case IPP_BUF_ENQUEUE:
>>> config = &property->config[EXYNOS_DRM_OPS_SRC];
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_Y],
>>> - EXYNOS_CIIYSA(buf_id));
>>> + EXYNOS_CIIYSA0);
>>>
>>> if (config->fmt == DRM_FORMAT_YVU420) {
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>>> - EXYNOS_CIICBSA(buf_id));
>>> + EXYNOS_CIICBSA0);
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>>> - EXYNOS_CIICRSA(buf_id));
>>> + EXYNOS_CIICRSA0);
>>> } else {
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CB],
>>> - EXYNOS_CIICBSA(buf_id));
>>> + EXYNOS_CIICBSA0);
>>> fimc_write(ctx, buf_info->base[EXYNOS_DRM_PLANAR_CR],
>>> - EXYNOS_CIICRSA(buf_id));
>>> + EXYNOS_CIICRSA0);
>>> }
>>> break;
>>> case IPP_BUF_DEQUEUE:
>>> - fimc_write(ctx, 0x0, EXYNOS_CIIYSA(buf_id));
>>> - fimc_write(ctx, 0x0, EXYNOS_CIICBSA(buf_id));
>>> - fimc_write(ctx, 0x0, EXYNOS_CIICRSA(buf_id));
>>> + fimc_write(ctx, 0x0, EXYNOS_CIIYSA0);
>>> + fimc_write(ctx, 0x0, EXYNOS_CIICBSA0);
>>> + fimc_write(ctx, 0x0, EXYNOS_CIICRSA0);
>>> break;
>>> default:
>>> /* bypass */
>>>
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 00/15] drm/exynos/ipp: image post processing fixes and improvements, part four
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-26 6:52 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 6:52 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> This set of patches contains various improvement and fixes
> for exynos_drm ipp framework.
> The patchset is based on exynos-drm-next branch.
>
> IPP framework was tested for regressions on exynos4210-trats target.
>
> Regards
> Andrzej
>
>
> Andrzej Hajda (15):
> drm/exynos/ipp: remove fake pm callbacks
> drm/exynos/ipp: cancel works before command node clean
> drm/exynos/ipp: move file reference from memory to command node
> drm/exynos/ipp: remove only related commands on file close
> drm/exynos/ipp: remove unused field in command node
> drm/exynos/ipp: free partially allocated resources on error
> drm/exynos/ipp: move nodes cleaning to separate function
> drm/exynos/ipp: clean memory nodes on command node cleaning
> drm/exynos/ipp: replace work_struct casting with better constructs
> drm/exynos/ipp: stop hardware before freeing memory
> drm/exynos/ipp: remove events during command cleaning
> drm/exynos/fimc: avoid clearing overflow bits
> drm/exynos/fimc: do not enable fimc twice
> drm/exynos/fimc: simplify buffer queuing
> drm/exynos/fimc: fix source buffer registers
With some minor comments,
Reviewed-by: Joonyoung Shim <jy0922.shim@samsung.com>
Thanks.
>
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 90 ++-----
> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 369 ++++++++++++----------------
> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
> drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +-
> 5 files changed, 180 insertions(+), 289 deletions(-)
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 00/15] drm/exynos/ipp: image post processing fixes and improvements, part four
@ 2014-08-26 6:52 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-26 6:52 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
> This set of patches contains various improvement and fixes
> for exynos_drm ipp framework.
> The patchset is based on exynos-drm-next branch.
>
> IPP framework was tested for regressions on exynos4210-trats target.
>
> Regards
> Andrzej
>
>
> Andrzej Hajda (15):
> drm/exynos/ipp: remove fake pm callbacks
> drm/exynos/ipp: cancel works before command node clean
> drm/exynos/ipp: move file reference from memory to command node
> drm/exynos/ipp: remove only related commands on file close
> drm/exynos/ipp: remove unused field in command node
> drm/exynos/ipp: free partially allocated resources on error
> drm/exynos/ipp: move nodes cleaning to separate function
> drm/exynos/ipp: clean memory nodes on command node cleaning
> drm/exynos/ipp: replace work_struct casting with better constructs
> drm/exynos/ipp: stop hardware before freeing memory
> drm/exynos/ipp: remove events during command cleaning
> drm/exynos/fimc: avoid clearing overflow bits
> drm/exynos/fimc: do not enable fimc twice
> drm/exynos/fimc: simplify buffer queuing
> drm/exynos/fimc: fix source buffer registers
With some minor comments,
Reviewed-by: Joonyoung Shim <jy0922.shim@samsung.com>
Thanks.
>
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 90 ++-----
> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 369 ++++++++++++----------------
> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
> drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +-
> 5 files changed, 180 insertions(+), 289 deletions(-)
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
2014-08-26 5:00 ` Joonyoung Shim
@ 2014-08-27 10:27 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-27 10:27 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
On 08/26/2014 07:00 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> In case of allocation errors some already allocated buffers
>> were not freed. The patch fixes it.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
>> 1 file changed, 33 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> index 060a198..728f3b9 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> @@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
>> return ret;
>> }
>>
>> +static int ipp_put_mem_node(struct drm_device *drm_dev,
>> + struct drm_exynos_ipp_cmd_node *c_node,
>> + struct drm_exynos_ipp_mem_node *m_node)
>> +{
>> + int i;
>> +
>> + DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>> +
>> + if (!m_node) {
>> + DRM_ERROR("invalid dequeue node.\n");
>> + return -EFAULT;
>> + }
>> +
>> + DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>> +
>> + /* put gem buffer */
>> + for_each_ipp_planar(i) {
>> + unsigned long handle = m_node->buf_info.handles[i];
>> + if (handle)
>> + exynos_drm_gem_put_dma_addr(drm_dev, handle,
>> + c_node->filp);
>> + }
>> +
>> + /* conditionally remove from queue */
>> + if (m_node->list.next)
> How about do INIT_LIST_HEAD for list in ipp_get_mem_node()?
I am not sure if it is better. For sure it adds unnecessary
initialization sequence.
Maybe adding list_is_initialized inline function to list.h would be the
best solution.
Regards
Andrzej
>
>> + list_del(&m_node->list);
>> + kfree(m_node);
>> +
>> + return 0;
>> +}
>> +
>> static struct drm_exynos_ipp_mem_node
>> *ipp_get_mem_node(struct drm_device *drm_dev,
>> struct drm_file *file,
>> @@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
>> qbuf->handle[i], file);
>> if (IS_ERR(addr)) {
>> DRM_ERROR("failed to get addr.\n");
>> - goto err_clear;
>> + ipp_put_mem_node(drm_dev, c_node, m_node);
>> + return ERR_PTR(-EFAULT);
>> }
>>
>> buf_info->handles[i] = qbuf->handle[i];
>> @@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
>> mutex_unlock(&c_node->mem_lock);
>>
>> return m_node;
>> -
>> -err_clear:
>> - kfree(m_node);
>> - return ERR_PTR(-EFAULT);
>> -}
>> -
>> -static int ipp_put_mem_node(struct drm_device *drm_dev,
>> - struct drm_exynos_ipp_cmd_node *c_node,
>> - struct drm_exynos_ipp_mem_node *m_node)
>> -{
>> - int i;
>> -
>> - DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>> -
>> - if (!m_node) {
>> - DRM_ERROR("invalid dequeue node.\n");
>> - return -EFAULT;
>> - }
>> -
>> - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>> -
>> - /* put gem buffer */
>> - for_each_ipp_planar(i) {
>> - unsigned long handle = m_node->buf_info.handles[i];
>> - if (handle)
>> - exynos_drm_gem_put_dma_addr(drm_dev, handle,
>> - c_node->filp);
>> - }
>> -
>> - /* delete list in queue */
>> - list_del(&m_node->list);
>> - kfree(m_node);
>> -
>> - return 0;
>> }
>>
>> static void ipp_free_event(struct drm_pending_event *event)
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
@ 2014-08-27 10:27 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-27 10:27 UTC (permalink / raw)
To: Joonyoung Shim, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...
On 08/26/2014 07:00 AM, Joonyoung Shim wrote:
> Hi Andrzej,
>
> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>> In case of allocation errors some already allocated buffers
>> were not freed. The patch fixes it.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
>> 1 file changed, 33 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> index 060a198..728f3b9 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>> @@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
>> return ret;
>> }
>>
>> +static int ipp_put_mem_node(struct drm_device *drm_dev,
>> + struct drm_exynos_ipp_cmd_node *c_node,
>> + struct drm_exynos_ipp_mem_node *m_node)
>> +{
>> + int i;
>> +
>> + DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>> +
>> + if (!m_node) {
>> + DRM_ERROR("invalid dequeue node.\n");
>> + return -EFAULT;
>> + }
>> +
>> + DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>> +
>> + /* put gem buffer */
>> + for_each_ipp_planar(i) {
>> + unsigned long handle = m_node->buf_info.handles[i];
>> + if (handle)
>> + exynos_drm_gem_put_dma_addr(drm_dev, handle,
>> + c_node->filp);
>> + }
>> +
>> + /* conditionally remove from queue */
>> + if (m_node->list.next)
> How about do INIT_LIST_HEAD for list in ipp_get_mem_node()?
I am not sure if it is better. For sure it adds unnecessary
initialization sequence.
Maybe adding list_is_initialized inline function to list.h would be the
best solution.
Regards
Andrzej
>
>> + list_del(&m_node->list);
>> + kfree(m_node);
>> +
>> + return 0;
>> +}
>> +
>> static struct drm_exynos_ipp_mem_node
>> *ipp_get_mem_node(struct drm_device *drm_dev,
>> struct drm_file *file,
>> @@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
>> qbuf->handle[i], file);
>> if (IS_ERR(addr)) {
>> DRM_ERROR("failed to get addr.\n");
>> - goto err_clear;
>> + ipp_put_mem_node(drm_dev, c_node, m_node);
>> + return ERR_PTR(-EFAULT);
>> }
>>
>> buf_info->handles[i] = qbuf->handle[i];
>> @@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
>> mutex_unlock(&c_node->mem_lock);
>>
>> return m_node;
>> -
>> -err_clear:
>> - kfree(m_node);
>> - return ERR_PTR(-EFAULT);
>> -}
>> -
>> -static int ipp_put_mem_node(struct drm_device *drm_dev,
>> - struct drm_exynos_ipp_cmd_node *c_node,
>> - struct drm_exynos_ipp_mem_node *m_node)
>> -{
>> - int i;
>> -
>> - DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>> -
>> - if (!m_node) {
>> - DRM_ERROR("invalid dequeue node.\n");
>> - return -EFAULT;
>> - }
>> -
>> - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>> -
>> - /* put gem buffer */
>> - for_each_ipp_planar(i) {
>> - unsigned long handle = m_node->buf_info.handles[i];
>> - if (handle)
>> - exynos_drm_gem_put_dma_addr(drm_dev, handle,
>> - c_node->filp);
>> - }
>> -
>> - /* delete list in queue */
>> - list_del(&m_node->list);
>> - kfree(m_node);
>> -
>> - return 0;
>> }
>>
>> static void ipp_free_event(struct drm_pending_event *event)
>>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 14/15] drm/exynos/fimc: simplify buffer queuing
2014-08-22 7:52 ` Andrzej Hajda
@ 2014-08-27 11:07 ` Andrzej Hajda
-1 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-27 11:07 UTC (permalink / raw)
To: Inki Dae
Cc: Andrzej Hajda, Marek Szyprowski, Joonyoung Shim, Seung-Woo Kim,
Kyungmin Park, dri-devel, open list,
moderated list:ARM/S5P EXYNOS AR...
The patch removes redundant checks, redundant HW reads
and simplifies code.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2:
- fixed bit cleaning operation
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
1 file changed, 15 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 62ba033..59ba8bb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1126,67 +1126,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
return 0;
}
-static int fimc_dst_get_buf_count(struct fimc_context *ctx)
-{
- u32 cfg, buf_num;
-
- cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
-
- buf_num = hweight32(cfg);
-
- DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
-
- return buf_num;
-}
-
-static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
+static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
enum drm_exynos_ipp_buf_type buf_type)
{
- struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
- bool enable;
- u32 cfg;
- u32 mask = 0x00000001 << buf_id;
- int ret = 0;
unsigned long flags;
+ u32 buf_num;
+ u32 cfg;
DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
spin_lock_irqsave(&ctx->lock, flags);
- /* mask register set */
cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
- switch (buf_type) {
- case IPP_BUF_ENQUEUE:
- enable = true;
- break;
- case IPP_BUF_DEQUEUE:
- enable = false;
- break;
- default:
- dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
- ret = -EINVAL;
- goto err_unlock;
- }
+ if (buf_type == IPP_BUF_ENQUEUE)
+ cfg |= (1 << buf_id);
+ else
+ cfg &= ~(1 << buf_id);
- /* sequence id */
- cfg &= ~mask;
- cfg |= (enable << buf_id);
fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
- /* interrupt enable */
- if (buf_type == IPP_BUF_ENQUEUE &&
- fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
- fimc_mask_irq(ctx, true);
+ buf_num = hweight32(cfg);
- /* interrupt disable */
- if (buf_type == IPP_BUF_DEQUEUE &&
- fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
+ if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
+ fimc_mask_irq(ctx, true);
+ else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
fimc_mask_irq(ctx, false);
-err_unlock:
spin_unlock_irqrestore(&ctx->lock, flags);
- return ret;
}
static int fimc_dst_set_addr(struct device *dev,
@@ -1244,7 +1211,9 @@ static int fimc_dst_set_addr(struct device *dev,
break;
}
- return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+ fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+
+ return 0;
}
static struct exynos_drm_ipp_ops fimc_dst_ops = {
@@ -1299,10 +1268,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
- if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
- DRM_ERROR("failed to dequeue.\n");
- return IRQ_HANDLED;
- }
+ fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH v2 14/15] drm/exynos/fimc: simplify buffer queuing
@ 2014-08-27 11:07 ` Andrzej Hajda
0 siblings, 0 replies; 58+ messages in thread
From: Andrzej Hajda @ 2014-08-27 11:07 UTC (permalink / raw)
To: Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Andrzej Hajda,
Kyungmin Park, Marek Szyprowski
The patch removes redundant checks, redundant HW reads
and simplifies code.
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2:
- fixed bit cleaning operation
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 64 ++++++++------------------------
1 file changed, 15 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 62ba033..59ba8bb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -1126,67 +1126,34 @@ static int fimc_dst_set_size(struct device *dev, int swap,
return 0;
}
-static int fimc_dst_get_buf_count(struct fimc_context *ctx)
-{
- u32 cfg, buf_num;
-
- cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
-
- buf_num = hweight32(cfg);
-
- DRM_DEBUG_KMS("buf_num[%d]\n", buf_num);
-
- return buf_num;
-}
-
-static int fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
+static void fimc_dst_set_buf_seq(struct fimc_context *ctx, u32 buf_id,
enum drm_exynos_ipp_buf_type buf_type)
{
- struct exynos_drm_ippdrv *ippdrv = &ctx->ippdrv;
- bool enable;
- u32 cfg;
- u32 mask = 0x00000001 << buf_id;
- int ret = 0;
unsigned long flags;
+ u32 buf_num;
+ u32 cfg;
DRM_DEBUG_KMS("buf_id[%d]buf_type[%d]\n", buf_id, buf_type);
spin_lock_irqsave(&ctx->lock, flags);
- /* mask register set */
cfg = fimc_read(ctx, EXYNOS_CIFCNTSEQ);
- switch (buf_type) {
- case IPP_BUF_ENQUEUE:
- enable = true;
- break;
- case IPP_BUF_DEQUEUE:
- enable = false;
- break;
- default:
- dev_err(ippdrv->dev, "invalid buf ctrl parameter.\n");
- ret = -EINVAL;
- goto err_unlock;
- }
+ if (buf_type == IPP_BUF_ENQUEUE)
+ cfg |= (1 << buf_id);
+ else
+ cfg &= ~(1 << buf_id);
- /* sequence id */
- cfg &= ~mask;
- cfg |= (enable << buf_id);
fimc_write(ctx, cfg, EXYNOS_CIFCNTSEQ);
- /* interrupt enable */
- if (buf_type == IPP_BUF_ENQUEUE &&
- fimc_dst_get_buf_count(ctx) >= FIMC_BUF_START)
- fimc_mask_irq(ctx, true);
+ buf_num = hweight32(cfg);
- /* interrupt disable */
- if (buf_type == IPP_BUF_DEQUEUE &&
- fimc_dst_get_buf_count(ctx) <= FIMC_BUF_STOP)
+ if (buf_type == IPP_BUF_ENQUEUE && buf_num >= FIMC_BUF_START)
+ fimc_mask_irq(ctx, true);
+ else if (buf_type == IPP_BUF_DEQUEUE && buf_num <= FIMC_BUF_STOP)
fimc_mask_irq(ctx, false);
-err_unlock:
spin_unlock_irqrestore(&ctx->lock, flags);
- return ret;
}
static int fimc_dst_set_addr(struct device *dev,
@@ -1244,7 +1211,9 @@ static int fimc_dst_set_addr(struct device *dev,
break;
}
- return fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+ fimc_dst_set_buf_seq(ctx, buf_id, buf_type);
+
+ return 0;
}
static struct exynos_drm_ipp_ops fimc_dst_ops = {
@@ -1299,10 +1268,7 @@ static irqreturn_t fimc_irq_handler(int irq, void *dev_id)
DRM_DEBUG_KMS("buf_id[%d]\n", buf_id);
- if (fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE) < 0) {
- DRM_ERROR("failed to dequeue.\n");
- return IRQ_HANDLED;
- }
+ fimc_dst_set_buf_seq(ctx, buf_id, IPP_BUF_DEQUEUE);
event_work->ippdrv = ippdrv;
event_work->buf_id[EXYNOS_DRM_OPS_DST] = buf_id;
--
1.9.1
^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
2014-08-27 10:27 ` Andrzej Hajda
@ 2014-08-27 23:59 ` Joonyoung Shim
-1 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-27 23:59 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: Marek Szyprowski, Seung-Woo Kim, Kyungmin Park, dri-devel,
open list, moderated list:ARM/S5P EXYNOS AR...,
Joonyoung Shim
Hi,
On 08/27/2014 07:27 PM, Andrzej Hajda wrote:
> On 08/26/2014 07:00 AM, Joonyoung Shim wrote:
>> Hi Andrzej,
>>
>> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>>> In case of allocation errors some already allocated buffers
>>> were not freed. The patch fixes it.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
>>> 1 file changed, 33 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> index 060a198..728f3b9 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> @@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
>>> return ret;
>>> }
>>>
>>> +static int ipp_put_mem_node(struct drm_device *drm_dev,
>>> + struct drm_exynos_ipp_cmd_node *c_node,
>>> + struct drm_exynos_ipp_mem_node *m_node)
>>> +{
>>> + int i;
>>> +
>>> + DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>>> +
>>> + if (!m_node) {
>>> + DRM_ERROR("invalid dequeue node.\n");
>>> + return -EFAULT;
>>> + }
>>> +
>>> + DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>>> +
>>> + /* put gem buffer */
>>> + for_each_ipp_planar(i) {
>>> + unsigned long handle = m_node->buf_info.handles[i];
>>> + if (handle)
>>> + exynos_drm_gem_put_dma_addr(drm_dev, handle,
>>> + c_node->filp);
>>> + }
>>> +
>>> + /* conditionally remove from queue */
>>> + if (m_node->list.next)
>> How about do INIT_LIST_HEAD for list in ipp_get_mem_node()?
>
> I am not sure if it is better. For sure it adds unnecessary
> initialization sequence.
In other words, this NULL checking is unnecessary if you initialize the
list_head by INIT_LIST_HEAD.
> Maybe adding list_is_initialized inline function to list.h would be the
> best solution.
There is just list_empty and we can't know whether list is initialized
or not. I recommend to use initialized list_head.
Thanks.
>
> Regards
> Andrzej
>
>>
>>> + list_del(&m_node->list);
>>> + kfree(m_node);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct drm_exynos_ipp_mem_node
>>> *ipp_get_mem_node(struct drm_device *drm_dev,
>>> struct drm_file *file,
>>> @@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
>>> qbuf->handle[i], file);
>>> if (IS_ERR(addr)) {
>>> DRM_ERROR("failed to get addr.\n");
>>> - goto err_clear;
>>> + ipp_put_mem_node(drm_dev, c_node, m_node);
>>> + return ERR_PTR(-EFAULT);
>>> }
>>>
>>> buf_info->handles[i] = qbuf->handle[i];
>>> @@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
>>> mutex_unlock(&c_node->mem_lock);
>>>
>>> return m_node;
>>> -
>>> -err_clear:
>>> - kfree(m_node);
>>> - return ERR_PTR(-EFAULT);
>>> -}
>>> -
>>> -static int ipp_put_mem_node(struct drm_device *drm_dev,
>>> - struct drm_exynos_ipp_cmd_node *c_node,
>>> - struct drm_exynos_ipp_mem_node *m_node)
>>> -{
>>> - int i;
>>> -
>>> - DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>>> -
>>> - if (!m_node) {
>>> - DRM_ERROR("invalid dequeue node.\n");
>>> - return -EFAULT;
>>> - }
>>> -
>>> - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>>> -
>>> - /* put gem buffer */
>>> - for_each_ipp_planar(i) {
>>> - unsigned long handle = m_node->buf_info.handles[i];
>>> - if (handle)
>>> - exynos_drm_gem_put_dma_addr(drm_dev, handle,
>>> - c_node->filp);
>>> - }
>>> -
>>> - /* delete list in queue */
>>> - list_del(&m_node->list);
>>> - kfree(m_node);
>>> -
>>> - return 0;
>>> }
>>>
>>> static void ipp_free_event(struct drm_pending_event *event)
>>>
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error
@ 2014-08-27 23:59 ` Joonyoung Shim
0 siblings, 0 replies; 58+ messages in thread
From: Joonyoung Shim @ 2014-08-27 23:59 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae
Cc: moderated list:ARM/S5P EXYNOS AR...,
Seung-Woo Kim, open list, dri-devel, Kyungmin Park,
Marek Szyprowski
Hi,
On 08/27/2014 07:27 PM, Andrzej Hajda wrote:
> On 08/26/2014 07:00 AM, Joonyoung Shim wrote:
>> Hi Andrzej,
>>
>> On 08/22/2014 04:52 PM, Andrzej Hajda wrote:
>>> In case of allocation errors some already allocated buffers
>>> were not freed. The patch fixes it.
>>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 68 ++++++++++++++++-----------------
>>> 1 file changed, 33 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> index 060a198..728f3b9 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
>>> @@ -599,6 +599,37 @@ static int ipp_set_mem_node(struct exynos_drm_ippdrv *ippdrv,
>>> return ret;
>>> }
>>>
>>> +static int ipp_put_mem_node(struct drm_device *drm_dev,
>>> + struct drm_exynos_ipp_cmd_node *c_node,
>>> + struct drm_exynos_ipp_mem_node *m_node)
>>> +{
>>> + int i;
>>> +
>>> + DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>>> +
>>> + if (!m_node) {
>>> + DRM_ERROR("invalid dequeue node.\n");
>>> + return -EFAULT;
>>> + }
>>> +
>>> + DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>>> +
>>> + /* put gem buffer */
>>> + for_each_ipp_planar(i) {
>>> + unsigned long handle = m_node->buf_info.handles[i];
>>> + if (handle)
>>> + exynos_drm_gem_put_dma_addr(drm_dev, handle,
>>> + c_node->filp);
>>> + }
>>> +
>>> + /* conditionally remove from queue */
>>> + if (m_node->list.next)
>> How about do INIT_LIST_HEAD for list in ipp_get_mem_node()?
>
> I am not sure if it is better. For sure it adds unnecessary
> initialization sequence.
In other words, this NULL checking is unnecessary if you initialize the
list_head by INIT_LIST_HEAD.
> Maybe adding list_is_initialized inline function to list.h would be the
> best solution.
There is just list_empty and we can't know whether list is initialized
or not. I recommend to use initialized list_head.
Thanks.
>
> Regards
> Andrzej
>
>>
>>> + list_del(&m_node->list);
>>> + kfree(m_node);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static struct drm_exynos_ipp_mem_node
>>> *ipp_get_mem_node(struct drm_device *drm_dev,
>>> struct drm_file *file,
>>> @@ -634,7 +665,8 @@ static struct drm_exynos_ipp_mem_node
>>> qbuf->handle[i], file);
>>> if (IS_ERR(addr)) {
>>> DRM_ERROR("failed to get addr.\n");
>>> - goto err_clear;
>>> + ipp_put_mem_node(drm_dev, c_node, m_node);
>>> + return ERR_PTR(-EFAULT);
>>> }
>>>
>>> buf_info->handles[i] = qbuf->handle[i];
>>> @@ -649,40 +681,6 @@ static struct drm_exynos_ipp_mem_node
>>> mutex_unlock(&c_node->mem_lock);
>>>
>>> return m_node;
>>> -
>>> -err_clear:
>>> - kfree(m_node);
>>> - return ERR_PTR(-EFAULT);
>>> -}
>>> -
>>> -static int ipp_put_mem_node(struct drm_device *drm_dev,
>>> - struct drm_exynos_ipp_cmd_node *c_node,
>>> - struct drm_exynos_ipp_mem_node *m_node)
>>> -{
>>> - int i;
>>> -
>>> - DRM_DEBUG_KMS("node[0x%x]\n", (int)m_node);
>>> -
>>> - if (!m_node) {
>>> - DRM_ERROR("invalid dequeue node.\n");
>>> - return -EFAULT;
>>> - }
>>> -
>>> - DRM_DEBUG_KMS("ops_id[%d]\n", m_node->ops_id);
>>> -
>>> - /* put gem buffer */
>>> - for_each_ipp_planar(i) {
>>> - unsigned long handle = m_node->buf_info.handles[i];
>>> - if (handle)
>>> - exynos_drm_gem_put_dma_addr(drm_dev, handle,
>>> - c_node->filp);
>>> - }
>>> -
>>> - /* delete list in queue */
>>> - list_del(&m_node->list);
>>> - kfree(m_node);
>>> -
>>> - return 0;
>>> }
>>>
>>> static void ipp_free_event(struct drm_pending_event *event)
>>>
>>
>
>
^ permalink raw reply [flat|nested] 58+ messages in thread
end of thread, other threads:[~2014-08-27 23:59 UTC | newest]
Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-22 7:52 [PATCH 00/15] drm/exynos/ipp: image post processing fixes and improvements, part four Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 01/15] drm/exynos/ipp: remove fake pm callbacks Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 02/15] drm/exynos/ipp: cancel works before command node clean Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 03/15] drm/exynos/ipp: move file reference from memory to command node Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-26 2:55 ` Joonyoung Shim
2014-08-26 2:55 ` Joonyoung Shim
2014-08-26 2:59 ` Joonyoung Shim
2014-08-26 2:59 ` Joonyoung Shim
2014-08-26 6:16 ` Andrzej Hajda
2014-08-26 6:16 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 04/15] drm/exynos/ipp: remove only related commands on file close Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 05/15] drm/exynos/ipp: remove unused field in command node Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 06/15] drm/exynos/ipp: free partially allocated resources on error Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-26 5:00 ` Joonyoung Shim
2014-08-26 5:00 ` Joonyoung Shim
2014-08-27 10:27 ` Andrzej Hajda
2014-08-27 10:27 ` Andrzej Hajda
2014-08-27 23:59 ` Joonyoung Shim
2014-08-27 23:59 ` Joonyoung Shim
2014-08-22 7:52 ` [PATCH 07/15] drm/exynos/ipp: move nodes cleaning to separate function Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 08/15] drm/exynos/ipp: clean memory nodes on command node cleaning Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 09/15] drm/exynos/ipp: replace work_struct casting with better constructs Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 10/15] drm/exynos/ipp: stop hardware before freeing memory Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 11/15] drm/exynos/ipp: remove events during command cleaning Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 12/15] drm/exynos/fimc: avoid clearing overflow bits Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 13/15] drm/exynos/fimc: do not enable fimc twice Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 14/15] drm/exynos/fimc: simplify buffer queuing Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-26 5:53 ` Joonyoung Shim
2014-08-26 5:53 ` Joonyoung Shim
2014-08-26 6:20 ` Andrzej Hajda
2014-08-26 6:20 ` Andrzej Hajda
2014-08-27 11:07 ` [PATCH v2 " Andrzej Hajda
2014-08-27 11:07 ` Andrzej Hajda
2014-08-22 7:52 ` [PATCH 15/15] drm/exynos/fimc: fix source buffer registers Andrzej Hajda
2014-08-22 7:52 ` Andrzej Hajda
2014-08-26 5:57 ` Joonyoung Shim
2014-08-26 5:57 ` Joonyoung Shim
2014-08-26 6:35 ` Andrzej Hajda
2014-08-26 6:35 ` Andrzej Hajda
2014-08-26 6:47 ` Joonyoung Shim
2014-08-26 6:47 ` Joonyoung Shim
2014-08-26 6:52 ` [PATCH 00/15] drm/exynos/ipp: image post processing fixes and improvements, part four Joonyoung Shim
2014-08-26 6:52 ` Joonyoung Shim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.