[PATCH 1/2] drm/exynos: ipp: fix wrong index referencing a config element

[PATCH 1/2] drm/exynos: ipp: fix wrong index referencing a config element

[Hyungwon Hwang]

Config depends on the opreation. So it must be referenced by an
operation id, not a property id.

Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_ipp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index b7f1cbc..54c5cf4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -486,8 +486,7 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
 	unsigned int bpp;
 	int i;
 
-	/* The property id should already be varified */
-	ipp_cfg = &c_node->property.config[m_node->prop_id];
+	ipp_cfg = &c_node->property.config[m_node->ops_id];
 	num_plane = drm_format_num_planes(ipp_cfg->fmt);
 
 	/**
-- 
1.9.1

[PATCH 2/2] drm/exynos: ipp: validate a GEM handle with multiple planes

[Hyungwon Hwang]

FIMC & GSC driver can calculate the offset of planes. So there are
use cases which IPP receives just one GEM handle of an image with
multiple plane. This patch extends ipp_validate_mem_node() to validate
this case.

Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_ipp.c | 51 ++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 54c5cf4..b3dc778 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -482,8 +482,8 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
 {
 	struct drm_exynos_ipp_config *ipp_cfg;
 	unsigned int num_plane;
-	unsigned long min_size, size;
-	unsigned int bpp;
+	unsigned long size, buf_size = 0, plane_size, img_size = 0;
+	unsigned int bpp, width, height;
 	int i;
 
 	ipp_cfg = &c_node->property.config[m_node->ops_id];
@@ -497,20 +497,45 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
 	 * but it seems more than enough
 	 */
 	for (i = 0; i < num_plane; ++i) {
-		if (!m_node->buf_info.handles[i]) {
-			DRM_ERROR("invalid handle for plane %d\n", i);
-			return -EINVAL;
-		}
+		width = ipp_cfg->sz.hsize;
+		height = ipp_cfg->sz.vsize;
 		bpp = drm_format_plane_cpp(ipp_cfg->fmt, i);
-		min_size = (ipp_cfg->sz.hsize * ipp_cfg->sz.vsize * bpp) >> 3;
-		size = exynos_drm_gem_get_size(drm_dev,
-					       m_node->buf_info.handles[i],
-					       c_node->filp);
-		if (min_size > size) {
-			DRM_ERROR("invalid size for plane %d\n", i);
-			return -EINVAL;
+
+		/*
+		 * The result of drm_format_plane_cpp() for chroma planes must
+		 * be used with drm_format_xxxx_chroma_subsampling() for
+		 * correct result.
+		 */
+		if (i > 0) {
+			width /= drm_format_horz_chroma_subsampling(
+								ipp_cfg->fmt);
+			height /= drm_format_vert_chroma_subsampling(
+								ipp_cfg->fmt);
 		}
+		plane_size = width * height * bpp;
+		img_size += plane_size;
+
+		if (m_node->buf_info.handles[i]) {
+			size = exynos_drm_gem_get_size(drm_dev,
+					m_node->buf_info.handles[i],
+					c_node->filp);
+			if (plane_size > size) {
+				DRM_ERROR(
+					"buffer %d is smaller than required\n",
+					i);
+				return -EINVAL;
+			}
+
+			buf_size += size;
+		}
+	}
+
+	if (buf_size < img_size) {
+		DRM_ERROR("size of buffers(%lu) is smaller than image(%lu)\n",
+			buf_size, img_size);
+		return -EINVAL;
 	}
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH 1/2] drm/exynos: ipp: fix wrong index referencing a config element

[Inki Dae]

On 2015년 06월 09일 12:45, Hyungwon Hwang wrote:
> Config depends on the opreation. So it must be referenced by an
> operation id, not a property id.

Applied.

Thanks,
Inki Dae

> 
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index b7f1cbc..54c5cf4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -486,8 +486,7 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
>  	unsigned int bpp;
>  	int i;
>  
> -	/* The property id should already be varified */
> -	ipp_cfg = &c_node->property.config[m_node->prop_id];
> +	ipp_cfg = &c_node->property.config[m_node->ops_id];
>  	num_plane = drm_format_num_planes(ipp_cfg->fmt);
>  
>  	/**
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/exynos: ipp: validate a GEM handle with multiple planes

[Inki Dae]

On 2015년 06월 09일 12:45, Hyungwon Hwang wrote:
> FIMC & GSC driver can calculate the offset of planes. So there are
> use cases which IPP receives just one GEM handle of an image with
> multiple plane. This patch extends ipp_validate_mem_node() to validate
> this case.

Applied.

Thanks,
Inki Dae

> 
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 51 ++++++++++++++++++++++++---------
>  1 file changed, 38 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 54c5cf4..b3dc778 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -482,8 +482,8 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
>  {
>  	struct drm_exynos_ipp_config *ipp_cfg;
>  	unsigned int num_plane;
> -	unsigned long min_size, size;
> -	unsigned int bpp;
> +	unsigned long size, buf_size = 0, plane_size, img_size = 0;
> +	unsigned int bpp, width, height;
>  	int i;
>  
>  	ipp_cfg = &c_node->property.config[m_node->ops_id];
> @@ -497,20 +497,45 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
>  	 * but it seems more than enough
>  	 */
>  	for (i = 0; i < num_plane; ++i) {
> -		if (!m_node->buf_info.handles[i]) {
> -			DRM_ERROR("invalid handle for plane %d\n", i);
> -			return -EINVAL;
> -		}
> +		width = ipp_cfg->sz.hsize;
> +		height = ipp_cfg->sz.vsize;
>  		bpp = drm_format_plane_cpp(ipp_cfg->fmt, i);
> -		min_size = (ipp_cfg->sz.hsize * ipp_cfg->sz.vsize * bpp) >> 3;
> -		size = exynos_drm_gem_get_size(drm_dev,
> -					       m_node->buf_info.handles[i],
> -					       c_node->filp);
> -		if (min_size > size) {
> -			DRM_ERROR("invalid size for plane %d\n", i);
> -			return -EINVAL;
> +
> +		/*
> +		 * The result of drm_format_plane_cpp() for chroma planes must
> +		 * be used with drm_format_xxxx_chroma_subsampling() for
> +		 * correct result.
> +		 */
> +		if (i > 0) {
> +			width /= drm_format_horz_chroma_subsampling(
> +								ipp_cfg->fmt);
> +			height /= drm_format_vert_chroma_subsampling(
> +								ipp_cfg->fmt);
>  		}
> +		plane_size = width * height * bpp;
> +		img_size += plane_size;
> +
> +		if (m_node->buf_info.handles[i]) {
> +			size = exynos_drm_gem_get_size(drm_dev,
> +					m_node->buf_info.handles[i],
> +					c_node->filp);
> +			if (plane_size > size) {
> +				DRM_ERROR(
> +					"buffer %d is smaller than required\n",
> +					i);
> +				return -EINVAL;
> +			}
> +
> +			buf_size += size;
> +		}
> +	}
> +
> +	if (buf_size < img_size) {
> +		DRM_ERROR("size of buffers(%lu) is smaller than image(%lu)\n",
> +			buf_size, img_size);
> +		return -EINVAL;
>  	}
> +
>  	return 0;
>  }
>  
>