2014-07-03 13:10:52

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three

This set of independent patches contains various improvement and fixes
for exynos_drm ipp framework.
The patchset is based on exynos-drm-next branch.

Regards
Andrzej


Andrzej Hajda (12):
drm/exynos/ipp: remove type casting
drm/exynos/ipp: remove unused field from exynos_drm_ipp_private
drm/exynos/ipp: remove struct exynos_drm_ipp_private
drm/exynos/ipp: correct address type
drm/exynos/ipp: remove temporary variable
drm/exynos/ipp: remove incorrect checks of list_first_entry result
drm/exynos/ipp: simplify memory check function
drm/exynos/ipp: remove useless registration checks
drm/exynos/ipp: simplify ipp_find_obj
drm/exynos/ipp: remove redundant messages
drm/exynos/ipp: simplify ipp_create_id
drm/exynos/ipp: simplify ipp_find_driver

drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 +-
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 +++++++++-----------------------
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
3 files changed, 73 insertions(+), 197 deletions(-)

--
1.9.1


2014-07-03 13:10:58

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 01/12] drm/exynos/ipp: remove type casting

The patch replaces type casting with proper pointer.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index a1888e1..f3f114c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -432,7 +432,7 @@ static struct drm_exynos_ipp_event_work *ipp_create_event_work(void)
if (!event_work)
return ERR_PTR(-ENOMEM);

- INIT_WORK((struct work_struct *)event_work, ipp_sched_event);
+ INIT_WORK(&event_work->work, ipp_sched_event);

return event_work;
}
--
1.9.1

2014-07-03 13:11:02

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 03/12] drm/exynos/ipp: remove struct exynos_drm_ipp_private

struct exynos_drm_ipp_private contains only one pointer so all occurrences
of the struct can be replaced by the pointer itself.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 +-----
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 30 +++++++++---------------------
drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 ++--
3 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 651f37a..5f027f2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -236,13 +236,9 @@ struct exynos_drm_g2d_private {
struct list_head userptr_list;
};

-struct exynos_drm_ipp_private {
- struct device *dev;
-};
-
struct drm_exynos_file_private {
struct exynos_drm_g2d_private *g2d_priv;
- struct exynos_drm_ipp_private *ipp_priv;
+ struct device *ipp_dev;
struct file *anon_filp;
};

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 5fb89c02..34d185c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -308,8 +308,7 @@ int exynos_drm_ipp_get_property(struct drm_device *drm_dev, void *data,
struct drm_file *file)
{
struct drm_exynos_file_private *file_priv = file->driver_priv;
- struct exynos_drm_ipp_private *priv = file_priv->ipp_priv;
- struct device *dev = priv->dev;
+ struct device *dev = file_priv->ipp_dev;
struct ipp_context *ctx = get_ipp_context(dev);
struct drm_exynos_ipp_prop_list *prop_list = data;
struct exynos_drm_ippdrv *ippdrv;
@@ -441,8 +440,7 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
struct drm_file *file)
{
struct drm_exynos_file_private *file_priv = file->driver_priv;
- struct exynos_drm_ipp_private *priv = file_priv->ipp_priv;
- struct device *dev = priv->dev;
+ struct device *dev = file_priv->ipp_dev;
struct ipp_context *ctx = get_ipp_context(dev);
struct drm_exynos_ipp_property *property = data;
struct exynos_drm_ippdrv *ippdrv;
@@ -501,7 +499,7 @@ 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->priv = priv;
+ c_node->dev = dev;
c_node->property = *property;
c_node->state = IPP_STATE_IDLE;

@@ -929,8 +927,7 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, void *data,
struct drm_file *file)
{
struct drm_exynos_file_private *file_priv = file->driver_priv;
- struct exynos_drm_ipp_private *priv = file_priv->ipp_priv;
- struct device *dev = priv->dev;
+ struct device *dev = file_priv->ipp_dev;
struct ipp_context *ctx = get_ipp_context(dev);
struct drm_exynos_ipp_queue_buf *qbuf = data;
struct drm_exynos_ipp_cmd_node *c_node;
@@ -1061,9 +1058,8 @@ int exynos_drm_ipp_cmd_ctrl(struct drm_device *drm_dev, void *data,
struct drm_file *file)
{
struct drm_exynos_file_private *file_priv = file->driver_priv;
- struct exynos_drm_ipp_private *priv = file_priv->ipp_priv;
struct exynos_drm_ippdrv *ippdrv = NULL;
- struct device *dev = priv->dev;
+ struct device *dev = file_priv->ipp_dev;
struct ipp_context *ctx = get_ipp_context(dev);
struct drm_exynos_ipp_cmd_ctrl *cmd_ctrl = data;
struct drm_exynos_ipp_cmd_work *cmd_work;
@@ -1775,16 +1771,10 @@ static int ipp_subdrv_open(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_ipp_private *priv;
-
- priv = kzalloc(sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
- priv->dev = dev;
- file_priv->ipp_priv = priv;

+ file_priv->ipp_dev = dev;

- DRM_DEBUG_KMS("done priv[0x%x]\n", (int)priv);
+ DRM_DEBUG_KMS("done priv[0x%x]\n", (int)dev);

return 0;
}
@@ -1793,13 +1783,12 @@ 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_ipp_private *priv = file_priv->ipp_priv;
struct exynos_drm_ippdrv *ippdrv = NULL;
struct ipp_context *ctx = get_ipp_context(dev);
struct drm_exynos_ipp_cmd_node *c_node, *tc_node;
int count = 0;

- DRM_DEBUG_KMS("for priv[0x%x]\n", (int)priv);
+ 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);
@@ -1808,7 +1797,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->priv == priv) {
+ if (c_node->dev == file_priv->ipp_dev) {
/*
* userland goto unnormal state. process killed.
* and close the file.
@@ -1830,7 +1819,6 @@ static void ipp_subdrv_close(struct drm_device *drm_dev, struct device *dev,
mutex_unlock(&ippdrv->cmd_lock);
}

- kfree(priv);
return;
}

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.h b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
index 7aaeaae..6f48d62 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.h
@@ -48,7 +48,7 @@ struct drm_exynos_ipp_cmd_work {
/*
* A structure of command node.
*
- * @priv: IPP private information.
+ * @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.
@@ -64,7 +64,7 @@ struct drm_exynos_ipp_cmd_work {
* @state: state of command node.
*/
struct drm_exynos_ipp_cmd_node {
- struct exynos_drm_ipp_private *priv;
+ struct device *dev;
struct list_head list;
struct list_head event_list;
struct list_head mem_list[EXYNOS_DRM_OPS_MAX];
--
1.9.1

2014-07-03 13:11:07

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 06/12] drm/exynos/ipp: remove incorrect checks of list_first_entry result

list_first_entry does not return NULL on empty list so this check
does not make sense. Moreover there is already code which prevents calling
list_first_entry on empty lists.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 15 ---------------
1 file changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index f84ef8a..89ff7e3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1277,11 +1277,6 @@ static int ipp_start_property(struct exynos_drm_ippdrv *ippdrv,

m_node = list_first_entry(head,
struct drm_exynos_ipp_mem_node, list);
- if (!m_node) {
- DRM_ERROR("failed to get node.\n");
- ret = -EFAULT;
- goto err_unlock;
- }

DRM_DEBUG_KMS("m_node[0x%x]\n", (int)m_node);

@@ -1539,11 +1534,6 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv,

m_node = list_first_entry(head,
struct drm_exynos_ipp_mem_node, list);
- if (!m_node) {
- DRM_ERROR("empty memory node.\n");
- ret = -ENOMEM;
- goto err_mem_unlock;
- }

tbuf_id[i] = m_node->buf_id;
DRM_DEBUG_KMS("%s buf_id[%d]\n",
@@ -1580,11 +1570,6 @@ static int ipp_send_event(struct exynos_drm_ippdrv *ippdrv,

m_node = list_first_entry(head,
struct drm_exynos_ipp_mem_node, list);
- if (!m_node) {
- DRM_ERROR("empty memory node.\n");
- ret = -ENOMEM;
- goto err_mem_unlock;
- }

tbuf_id[EXYNOS_DRM_OPS_SRC] = m_node->buf_id;

--
1.9.1

2014-07-03 13:11:16

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 10/12] drm/exynos/ipp: remove redundant messages

In case of error callback prints already corresponding message.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index c7ea047..0552f62 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -1152,7 +1152,6 @@ static int ipp_set_property(struct exynos_drm_ippdrv *ippdrv,
/* reset h/w block */
if (ippdrv->reset &&
ippdrv->reset(ippdrv->dev)) {
- DRM_ERROR("failed to reset.\n");
return -EINVAL;
}

@@ -1170,30 +1169,24 @@ static int ipp_set_property(struct exynos_drm_ippdrv *ippdrv,
/* set format */
if (ops->set_fmt) {
ret = ops->set_fmt(ippdrv->dev, config->fmt);
- if (ret) {
- DRM_ERROR("not support format.\n");
+ if (ret)
return ret;
- }
}

/* set transform for rotation, flip */
if (ops->set_transf) {
ret = ops->set_transf(ippdrv->dev, config->degree,
config->flip, &swap);
- if (ret) {
- DRM_ERROR("not support tranf.\n");
- return -EINVAL;
- }
+ if (ret)
+ return ret;
}

/* set size */
if (ops->set_size) {
ret = ops->set_size(ippdrv->dev, swap, &config->pos,
&config->sz);
- if (ret) {
- DRM_ERROR("not support size.\n");
+ if (ret)
return ret;
- }
}
}

--
1.9.1

2014-07-03 13:11:12

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 08/12] drm/exynos/ipp: remove useless registration checks

Argument checks are redundant, clients always check ippdrv before calling
these functions.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index b7ce14e..26c8a2c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -129,9 +129,6 @@ void exynos_platform_device_ipp_unregister(void)

int exynos_drm_ippdrv_register(struct exynos_drm_ippdrv *ippdrv)
{
- if (!ippdrv)
- return -EINVAL;
-
mutex_lock(&exynos_drm_ippdrv_lock);
list_add_tail(&ippdrv->drv_list, &exynos_drm_ippdrv_list);
mutex_unlock(&exynos_drm_ippdrv_lock);
@@ -141,9 +138,6 @@ int exynos_drm_ippdrv_register(struct exynos_drm_ippdrv *ippdrv)

int exynos_drm_ippdrv_unregister(struct exynos_drm_ippdrv *ippdrv)
{
- if (!ippdrv)
- return -EINVAL;
-
mutex_lock(&exynos_drm_ippdrv_lock);
list_del(&ippdrv->drv_list);
mutex_unlock(&exynos_drm_ippdrv_lock);
--
1.9.1

2014-07-03 13:11:30

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 12/12] drm/exynos/ipp: simplify ipp_find_driver

The patch puts repeated code sequence into one function, removes verbose
comments and decreases log verbosity.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 74 ++++++++++-----------------------
1 file changed, 21 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index ae75a1d..c411399 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -174,18 +174,18 @@ static void *ipp_find_obj(struct idr *id_idr, struct mutex *lock, u32 id)
return obj;
}

-static inline bool ipp_check_dedicated(struct exynos_drm_ippdrv *ippdrv,
- enum drm_exynos_ipp_cmd cmd)
+static int ipp_check_driver(struct exynos_drm_ippdrv *ippdrv,
+ struct drm_exynos_ipp_property *property)
{
- /*
- * check dedicated flag and WB, OUTPUT operation with
- * power on state.
- */
- if (ippdrv->dedicated || (!ipp_is_m2m_cmd(cmd) &&
- !pm_runtime_suspended(ippdrv->dev)))
- return true;
+ if (ippdrv->dedicated || (!ipp_is_m2m_cmd(property->cmd) &&
+ !pm_runtime_suspended(ippdrv->dev)))
+ return -EBUSY;

- return false;
+ if (ippdrv->check_property &&
+ ippdrv->check_property(ippdrv->dev, property))
+ return -EINVAL;
+
+ return 0;
}

static struct exynos_drm_ippdrv *ipp_find_driver(struct ipp_context *ctx,
@@ -193,62 +193,30 @@ static struct exynos_drm_ippdrv *ipp_find_driver(struct ipp_context *ctx,
{
struct exynos_drm_ippdrv *ippdrv;
u32 ipp_id = property->ipp_id;
-
- DRM_DEBUG_KMS("ipp_id[%d]\n", ipp_id);
+ int ret;

if (ipp_id) {
- /* find ipp driver using idr */
- ippdrv = ipp_find_obj(&ctx->ipp_idr, &ctx->ipp_lock,
- ipp_id);
+ ippdrv = ipp_find_obj(&ctx->ipp_idr, &ctx->ipp_lock, ipp_id);
if (!ippdrv) {
- DRM_ERROR("not found ipp%d driver.\n", ipp_id);
+ DRM_DEBUG("ipp%d driver not found\n", ipp_id);
return ERR_PTR(-ENODEV);
}

- /*
- * WB, OUTPUT opertion not supported multi-operation.
- * so, make dedicated state at set property ioctl.
- * when ipp driver finished operations, clear dedicated flags.
- */
- if (ipp_check_dedicated(ippdrv, property->cmd)) {
- DRM_ERROR("already used choose device.\n");
- return ERR_PTR(-EBUSY);
- }
-
- /*
- * This is necessary to find correct device in ipp drivers.
- * ipp drivers have different abilities,
- * so need to check property.
- */
- if (ippdrv->check_property &&
- ippdrv->check_property(ippdrv->dev, property)) {
- DRM_ERROR("not support property.\n");
- return ERR_PTR(-EINVAL);
+ ret = ipp_check_driver(ippdrv, property);
+ if (ret < 0) {
+ DRM_DEBUG("ipp%d driver check error %d\n", ipp_id, ret);
+ return ERR_PTR(ret);
}

return ippdrv;
} else {
- /*
- * This case is search all ipp driver for finding.
- * user application don't set ipp_id in this case,
- * so ipp subsystem search correct driver in driver list.
- */
list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) {
- if (ipp_check_dedicated(ippdrv, property->cmd)) {
- DRM_DEBUG_KMS("used device.\n");
- continue;
- }
-
- if (ippdrv->check_property &&
- ippdrv->check_property(ippdrv->dev, property)) {
- DRM_DEBUG_KMS("not support property.\n");
- continue;
- }
-
- return ippdrv;
+ ret = ipp_check_driver(ippdrv, property);
+ if (ret == 0)
+ return ippdrv;
}

- DRM_ERROR("not support ipp driver operations.\n");
+ DRM_DEBUG("cannot find driver suitable for given property.\n");
}

return ERR_PTR(-ENODEV);
--
1.9.1

2014-07-03 13:11:47

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 11/12] drm/exynos/ipp: simplify ipp_create_id

There is no gain in passing id by pointer to be filled.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 0552f62..ae75a1d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -145,20 +145,15 @@ int exynos_drm_ippdrv_unregister(struct exynos_drm_ippdrv *ippdrv)
return 0;
}

-static int ipp_create_id(struct idr *id_idr, struct mutex *lock, void *obj,
- u32 *idp)
+static int ipp_create_id(struct idr *id_idr, struct mutex *lock, void *obj)
{
int ret;

- /* do the allocation under our mutexlock */
mutex_lock(lock);
ret = idr_alloc(id_idr, obj, 1, 0, GFP_KERNEL);
mutex_unlock(lock);
- if (ret < 0)
- return ret;

- *idp = ret;
- return 0;
+ return ret;
}

static void ipp_remove_id(struct idr *id_idr, struct mutex *lock, u32 id)
@@ -471,13 +466,12 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
if (!c_node)
return -ENOMEM;

- /* create property id */
- ret = ipp_create_id(&ctx->prop_idr, &ctx->prop_lock, c_node,
- &property->prop_id);
- if (ret) {
+ ret = ipp_create_id(&ctx->prop_idr, &ctx->prop_lock, c_node);
+ if (ret < 0) {
DRM_ERROR("failed to create id.\n");
goto err_clear;
}
+ property->prop_id = ret;

DRM_DEBUG_KMS("created prop_id[%d]cmd[%d]ippdrv[0x%x]\n",
property->prop_id, property->cmd, (int)ippdrv);
@@ -1636,21 +1630,17 @@ static int ipp_subdrv_probe(struct drm_device *drm_dev, struct device *dev)

/* get ipp driver entry */
list_for_each_entry(ippdrv, &exynos_drm_ippdrv_list, drv_list) {
- u32 ipp_id;
-
ippdrv->drm_dev = drm_dev;

- ret = ipp_create_id(&ctx->ipp_idr, &ctx->ipp_lock, ippdrv,
- &ipp_id);
- if (ret || ipp_id == 0) {
+ ret = ipp_create_id(&ctx->ipp_idr, &ctx->ipp_lock, ippdrv);
+ if (ret < 0) {
DRM_ERROR("failed to create id.\n");
goto err;
}
+ ippdrv->prop_list.ipp_id = ret;

DRM_DEBUG_KMS("count[%d]ippdrv[0x%x]ipp_id[%d]\n",
- count++, (int)ippdrv, ipp_id);
-
- ippdrv->prop_list.ipp_id = ipp_id;
+ count++, (int)ippdrv, ret);

/* store parent device for node */
ippdrv->parent_dev = dev;
--
1.9.1

2014-07-03 13:12:17

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 09/12] drm/exynos/ipp: simplify ipp_find_obj

The patch simplifies ipp_find_obj and removes debug messages.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 26c8a2c..c7ea047 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -172,18 +172,8 @@ static void *ipp_find_obj(struct idr *id_idr, struct mutex *lock, u32 id)
{
void *obj;

- DRM_DEBUG_KMS("id[%d]\n", id);
-
mutex_lock(lock);
-
- /* find object using handle */
obj = idr_find(id_idr, id);
- if (!obj) {
- DRM_ERROR("failed to find object.\n");
- mutex_unlock(lock);
- return ERR_PTR(-ENODEV);
- }
-
mutex_unlock(lock);

return obj;
@@ -215,9 +205,9 @@ static struct exynos_drm_ippdrv *ipp_find_driver(struct ipp_context *ctx,
/* find ipp driver using idr */
ippdrv = ipp_find_obj(&ctx->ipp_idr, &ctx->ipp_lock,
ipp_id);
- if (IS_ERR(ippdrv)) {
+ if (!ippdrv) {
DRM_ERROR("not found ipp%d driver.\n", ipp_id);
- return ippdrv;
+ return ERR_PTR(-ENODEV);
}

/*
@@ -339,10 +329,10 @@ int exynos_drm_ipp_get_property(struct drm_device *drm_dev, void *data,
*/
ippdrv = ipp_find_obj(&ctx->ipp_idr, &ctx->ipp_lock,
prop_list->ipp_id);
- if (IS_ERR(ippdrv)) {
+ if (!ippdrv) {
DRM_ERROR("not found ipp%d driver.\n",
prop_list->ipp_id);
- return PTR_ERR(ippdrv);
+ return -ENODEV;
}

*prop_list = ippdrv->prop_list;
@@ -920,9 +910,9 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, void *data,
/* find command node */
c_node = ipp_find_obj(&ctx->prop_idr, &ctx->prop_lock,
qbuf->prop_id);
- if (IS_ERR(c_node)) {
+ if (!c_node) {
DRM_ERROR("failed to get command node.\n");
- return PTR_ERR(c_node);
+ return -ENODEV;
}

/* buffer control */
@@ -1055,9 +1045,9 @@ int exynos_drm_ipp_cmd_ctrl(struct drm_device *drm_dev, void *data,

c_node = ipp_find_obj(&ctx->prop_idr, &ctx->prop_lock,
cmd_ctrl->prop_id);
- if (IS_ERR(c_node)) {
+ if (!c_node) {
DRM_ERROR("invalid command node list.\n");
- return PTR_ERR(c_node);
+ return -ENODEV;
}

if (!exynos_drm_ipp_check_valid(ippdrv->dev, cmd_ctrl->ctrl,
--
1.9.1

2014-07-03 13:12:40

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 07/12] drm/exynos/ipp: simplify memory check function

The only thing function should check is if there are buffers in respective
queues.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 44 ++++++++-------------------------
1 file changed, 10 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 89ff7e3..b7ce14e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -574,42 +574,18 @@ static void ipp_clean_cmd_node(struct ipp_context *ctx,
kfree(c_node);
}

-static int ipp_check_mem_list(struct drm_exynos_ipp_cmd_node *c_node)
+static bool ipp_check_mem_list(struct drm_exynos_ipp_cmd_node *c_node)
{
- struct drm_exynos_ipp_property *property = &c_node->property;
- struct drm_exynos_ipp_mem_node *m_node;
- struct list_head *head;
- int ret, i, count[EXYNOS_DRM_OPS_MAX] = { 0, };
-
- for_each_ipp_ops(i) {
- /* source/destination memory list */
- head = &c_node->mem_list[i];
-
- /* find memory node entry */
- list_for_each_entry(m_node, head, list) {
- DRM_DEBUG_KMS("%s,count[%d]m_node[0x%x]\n",
- i ? "dst" : "src", count[i], (int)m_node);
- count[i]++;
- }
+ switch (c_node->property.cmd) {
+ case IPP_CMD_WB:
+ return !list_empty(&c_node->mem_list[EXYNOS_DRM_OPS_DST]);
+ case IPP_CMD_OUTPUT:
+ return !list_empty(&c_node->mem_list[EXYNOS_DRM_OPS_SRC]);
+ case IPP_CMD_M2M:
+ default:
+ return !list_empty(&c_node->mem_list[EXYNOS_DRM_OPS_SRC]) &&
+ !list_empty(&c_node->mem_list[EXYNOS_DRM_OPS_DST]);
}
-
- DRM_DEBUG_KMS("min[%d]max[%d]\n",
- min(count[EXYNOS_DRM_OPS_SRC], count[EXYNOS_DRM_OPS_DST]),
- max(count[EXYNOS_DRM_OPS_SRC], count[EXYNOS_DRM_OPS_DST]));
-
- /*
- * M2M operations should be need paired memory address.
- * so, need to check minimum count about src, dst.
- * other case not use paired memory, so use maximum count
- */
- if (ipp_is_m2m_cmd(property->cmd))
- ret = min(count[EXYNOS_DRM_OPS_SRC],
- count[EXYNOS_DRM_OPS_DST]);
- else
- ret = max(count[EXYNOS_DRM_OPS_SRC],
- count[EXYNOS_DRM_OPS_DST]);
-
- return ret;
}

static struct drm_exynos_ipp_mem_node
--
1.9.1

2014-07-03 13:12:58

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 05/12] drm/exynos/ipp: remove temporary variable

There is no reason to allocate intermediate variable.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index f3d8b5c..f84ef8a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -680,15 +680,14 @@ static struct drm_exynos_ipp_mem_node
struct drm_exynos_ipp_queue_buf *qbuf)
{
struct drm_exynos_ipp_mem_node *m_node;
- struct drm_exynos_ipp_buf_info buf_info;
+ 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);

- /* clear base address for error handling */
- memset(&buf_info, 0x0, sizeof(buf_info));
+ buf_info = &m_node->buf_info;

/* operations, buffer id */
m_node->ops_id = qbuf->ops_id;
@@ -712,15 +711,14 @@ static struct drm_exynos_ipp_mem_node
goto err_clear;
}

- buf_info.handles[i] = qbuf->handle[i];
- buf_info.base[i] = *addr;
- DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%x]\n",
- i, buf_info.base[i], (int)buf_info.handles[i]);
+ 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]);
}
}

m_node->filp = file;
- m_node->buf_info = buf_info;
mutex_lock(&c_node->mem_lock);
list_add_tail(&m_node->list, &c_node->mem_list[qbuf->ops_id]);
mutex_unlock(&c_node->mem_lock);
--
1.9.1

2014-07-03 13:13:22

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 04/12] drm/exynos/ipp: correct address type

exynos_drm_gem_get_dma_addr returns dma_addr_t, type casting to void* and
back is not necessary.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 34d185c..f3d8b5c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -681,7 +681,6 @@ static struct drm_exynos_ipp_mem_node
{
struct drm_exynos_ipp_mem_node *m_node;
struct drm_exynos_ipp_buf_info buf_info;
- void *addr;
int i;

m_node = kzalloc(sizeof(*m_node), GFP_KERNEL);
@@ -704,6 +703,8 @@ static struct drm_exynos_ipp_mem_node

/* 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)) {
@@ -712,7 +713,7 @@ static struct drm_exynos_ipp_mem_node
}

buf_info.handles[i] = qbuf->handle[i];
- buf_info.base[i] = *(dma_addr_t *) addr;
+ buf_info.base[i] = *addr;
DRM_DEBUG_KMS("i[%d]base[0x%x]hd[0x%x]\n",
i, buf_info.base[i], (int)buf_info.handles[i]);
}
--
1.9.1

2014-07-03 13:13:47

by Andrzej Hajda

[permalink] [raw]
Subject: [PATCH 02/12] drm/exynos/ipp: remove unused field from exynos_drm_ipp_private

The patch removes unused event_list field from struct exynos_drm_ipp_private.

Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 -
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2 --
2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 06cde45..651f37a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -238,7 +238,6 @@ struct exynos_drm_g2d_private {

struct exynos_drm_ipp_private {
struct device *dev;
- struct list_head event_list;
};

struct drm_exynos_file_private {
diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index f3f114c..5fb89c02 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -534,7 +534,6 @@ int exynos_drm_ipp_set_property(struct drm_device *drm_dev, void *data,
INIT_LIST_HEAD(&c_node->mem_list[i]);

INIT_LIST_HEAD(&c_node->event_list);
- list_splice_init(&priv->event_list, &c_node->event_list);
mutex_lock(&ippdrv->cmd_lock);
list_add_tail(&c_node->list, &ippdrv->cmd_list);
mutex_unlock(&ippdrv->cmd_lock);
@@ -1784,7 +1783,6 @@ static int ipp_subdrv_open(struct drm_device *drm_dev, struct device *dev,
priv->dev = dev;
file_priv->ipp_priv = priv;

- INIT_LIST_HEAD(&priv->event_list);

DRM_DEBUG_KMS("done priv[0x%x]\n", (int)priv);

--
1.9.1

Subject: Re: [PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three

On 2014년 07월 03일 22:10, Andrzej Hajda wrote:
> This set of independent patches contains various improvement and fixes
> for exynos_drm ipp framework.
> The patchset is based on exynos-drm-next branch.
>

Did you test ipp module using libdrm? If so, can you share the app? I
would try to test this patch series before merging them. Mainline libdrm
has no any ipp interfaces.

Thanks,
Inki Dae

> Regards
> Andrzej
>
>
> Andrzej Hajda (12):
> drm/exynos/ipp: remove type casting
> drm/exynos/ipp: remove unused field from exynos_drm_ipp_private
> drm/exynos/ipp: remove struct exynos_drm_ipp_private
> drm/exynos/ipp: correct address type
> drm/exynos/ipp: remove temporary variable
> drm/exynos/ipp: remove incorrect checks of list_first_entry result
> drm/exynos/ipp: simplify memory check function
> drm/exynos/ipp: remove useless registration checks
> drm/exynos/ipp: simplify ipp_find_obj
> drm/exynos/ipp: remove redundant messages
> drm/exynos/ipp: simplify ipp_create_id
> drm/exynos/ipp: simplify ipp_find_driver
>
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 +++++++++-----------------------
> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
> 3 files changed, 73 insertions(+), 197 deletions(-)
>

2014-07-21 09:23:31

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three

Hi Inki,

On 07/09/2014 04:47 PM, Inki Dae wrote:
> On 2014년 07월 03일 22:10, Andrzej Hajda wrote:
>> This set of independent patches contains various improvement and fixes
>> for exynos_drm ipp framework.
>> The patchset is based on exynos-drm-next branch.
>>
> Did you test ipp module using libdrm? If so, can you share the app? I
> would try to test this patch series before merging them. Mainline libdrm
> has no any ipp interfaces.

I have used ipptest program developed by you :), with minor changes.
More advanced tests are on my TODO list, but for this patchset ipptest
seems to be OK - the patches are mainly cleanup patches.

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (12):
>> drm/exynos/ipp: remove type casting
>> drm/exynos/ipp: remove unused field from exynos_drm_ipp_private
>> drm/exynos/ipp: remove struct exynos_drm_ipp_private
>> drm/exynos/ipp: correct address type
>> drm/exynos/ipp: remove temporary variable
>> drm/exynos/ipp: remove incorrect checks of list_first_entry result
>> drm/exynos/ipp: simplify memory check function
>> drm/exynos/ipp: remove useless registration checks
>> drm/exynos/ipp: simplify ipp_find_obj
>> drm/exynos/ipp: remove redundant messages
>> drm/exynos/ipp: simplify ipp_create_id
>> drm/exynos/ipp: simplify ipp_find_driver
>>
>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 +-
>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 +++++++++-----------------------
>> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
>> 3 files changed, 73 insertions(+), 197 deletions(-)
>>
>

Subject: Re: [PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three

On 2014년 07월 21일 18:23, Andrzej Hajda wrote:
> Hi Inki,
>
> On 07/09/2014 04:47 PM, Inki Dae wrote:
>> On 2014년 07월 03일 22:10, Andrzej Hajda wrote:
>>> This set of independent patches contains various improvement and fixes
>>> for exynos_drm ipp framework.
>>> The patchset is based on exynos-drm-next branch.
>>>
>> Did you test ipp module using libdrm? If so, can you share the app? I
>> would try to test this patch series before merging them. Mainline libdrm
>> has no any ipp interfaces.
>
> I have used ipptest program developed by you :), with minor changes.
> More advanced tests are on my TODO list, but for this patchset ipptest
> seems to be OK - the patches are mainly cleanup patches.
>


Did the ipptest work fine? I just tried to build the app so didn't test
the app. :) Got it, will merge this patch series.

Thanks,
Inki Dae

> Regards
> Andrzej
>
>>
>> Thanks,
>> Inki Dae
>>
>>> Regards
>>> Andrzej
>>>
>>>
>>> Andrzej Hajda (12):
>>> drm/exynos/ipp: remove type casting
>>> drm/exynos/ipp: remove unused field from exynos_drm_ipp_private
>>> drm/exynos/ipp: remove struct exynos_drm_ipp_private
>>> drm/exynos/ipp: correct address type
>>> drm/exynos/ipp: remove temporary variable
>>> drm/exynos/ipp: remove incorrect checks of list_first_entry result
>>> drm/exynos/ipp: simplify memory check function
>>> drm/exynos/ipp: remove useless registration checks
>>> drm/exynos/ipp: simplify ipp_find_obj
>>> drm/exynos/ipp: remove redundant messages
>>> drm/exynos/ipp: simplify ipp_create_id
>>> drm/exynos/ipp: simplify ipp_find_driver
>>>
>>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 +-
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 +++++++++-----------------------
>>> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
>>> 3 files changed, 73 insertions(+), 197 deletions(-)
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

Subject: Re: [PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three

On 2014년 07월 03일 22:10, Andrzej Hajda wrote:
> This set of independent patches contains various improvement and fixes
> for exynos_drm ipp framework.
> The patchset is based on exynos-drm-next branch.

Applied.

Thanks,
Inki Dae

>
> Regards
> Andrzej
>
>
> Andrzej Hajda (12):
> drm/exynos/ipp: remove type casting
> drm/exynos/ipp: remove unused field from exynos_drm_ipp_private
> drm/exynos/ipp: remove struct exynos_drm_ipp_private
> drm/exynos/ipp: correct address type
> drm/exynos/ipp: remove temporary variable
> drm/exynos/ipp: remove incorrect checks of list_first_entry result
> drm/exynos/ipp: simplify memory check function
> drm/exynos/ipp: remove useless registration checks
> drm/exynos/ipp: simplify ipp_find_obj
> drm/exynos/ipp: remove redundant messages
> drm/exynos/ipp: simplify ipp_create_id
> drm/exynos/ipp: simplify ipp_find_driver
>
> drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 +++++++++-----------------------
> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +-
> 3 files changed, 73 insertions(+), 197 deletions(-)
>