[PATCH 1/8] [media] s5p-jpeg: Add fmt_ver_flag field to the s5p_jpeg_variant structure

[PATCH 1/8] [media] s5p-jpeg: Add fmt_ver_flag field to the s5p_jpeg_variant structure

[Jacek Anaszewski]

Simplify the code by adding fmt_ver_flag field
to the s5p_jpeg_variant structure which allows
to avoid "if" statement in the s5p_jpeg_find_format
function.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |   11 ++++-------
 drivers/media/platform/s5p-jpeg/jpeg-core.h |    1 +
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 6db4d5e..9b0102d 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -959,7 +959,7 @@ static int s5p_jpeg_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
 static struct s5p_jpeg_fmt *s5p_jpeg_find_format(struct s5p_jpeg_ctx *ctx,
 				u32 pixelformat, unsigned int fmt_type)
 {
-	unsigned int k, fmt_flag, ver_flag;
+	unsigned int k, fmt_flag;
 
 	if (ctx->mode == S5P_JPEG_ENCODE)
 		fmt_flag = (fmt_type == FMT_TYPE_OUTPUT) ?
@@ -970,16 +970,11 @@ static struct s5p_jpeg_fmt *s5p_jpeg_find_format(struct s5p_jpeg_ctx *ctx,
 				SJPEG_FMT_FLAG_DEC_OUTPUT :
 				SJPEG_FMT_FLAG_DEC_CAPTURE;
 
-	if (ctx->jpeg->variant->version == SJPEG_S5P)
-		ver_flag = SJPEG_FMT_FLAG_S5P;
-	else
-		ver_flag = SJPEG_FMT_FLAG_EXYNOS4;
-
 	for (k = 0; k < ARRAY_SIZE(sjpeg_formats); k++) {
 		struct s5p_jpeg_fmt *fmt = &sjpeg_formats[k];
 		if (fmt->fourcc == pixelformat &&
 		    fmt->flags & fmt_flag &&
-		    fmt->flags & ver_flag) {
+		    fmt->flags & ctx->jpeg->variant->fmt_ver_flag) {
 			return fmt;
 		}
 	}
@@ -2103,11 +2098,13 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
 static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
 	.version	= SJPEG_S5P,
 	.jpeg_irq	= s5p_jpeg_irq,
+	.fmt_ver_flag	= SJPEG_FMT_FLAG_S5P,
 };
 
 static struct s5p_jpeg_variant exynos4_jpeg_drvdata = {
 	.version	= SJPEG_EXYNOS4,
 	.jpeg_irq	= exynos4_jpeg_irq,
+	.fmt_ver_flag	= SJPEG_FMT_FLAG_EXYNOS4,
 };
 
 static const struct of_device_id samsung_jpeg_match[] = {
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h b/drivers/media/platform/s5p-jpeg/jpeg-core.h
index f482dbf..c222436 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.h
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h
@@ -118,6 +118,7 @@ struct s5p_jpeg {
 
 struct s5p_jpeg_variant {
 	unsigned int	version;
+	unsigned int	fmt_ver_flag;
 	irqreturn_t	(*jpeg_irq)(int irq, void *priv);
 };
 
-- 
1.7.9.5

[PATCH 2/8] [media] s5p-jpeg: Perform fourcc downgrade only for Exynos4x12 SoCs

[Jacek Anaszewski]

Change the driver variant check from "is not S5PC210"
to "is Exynos4" while checking whether YUV format needs
to be downgraded in order to prevent upsampling which
is not supported by Exynos4 SoCs family.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 9b0102d..c675c90 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1070,7 +1070,7 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file, void *priv,
 	 * If this requirement is not met then downgrade the requested
 	 * capture format to the one with subsampling equal to the input jpeg.
 	 */
-	if ((ctx->jpeg->variant->version != SJPEG_S5P) &&
+	if ((ctx->jpeg->variant->version == SJPEG_EXYNOS4) &&
 	    (ctx->mode == S5P_JPEG_DECODE) &&
 	    (fmt->flags & SJPEG_FMT_NON_RGB) &&
 	    (fmt->subsampling < ctx->subsampling)) {
-- 
1.7.9.5

[PATCH 3/8] [media] s5p-jpeg: Add m2m_ops field to the s5p_jpeg_variant structure

[Jacek Anaszewski]

Simplify the code by adding m2m_ops field to the
s5p_jpeg_variant structure which allows to avoid
"if" statement in the s5p_jpeg_probe function.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |   12 ++++--------
 drivers/media/platform/s5p-jpeg/jpeg-core.h |    7 ++++---
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index c675c90..4f4dc81 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1566,7 +1566,7 @@ static struct v4l2_m2m_ops s5p_jpeg_m2m_ops = {
 	.job_abort	= s5p_jpeg_job_abort,
 }
 ;
-static struct v4l2_m2m_ops exynos_jpeg_m2m_ops = {
+static struct v4l2_m2m_ops exynos4_jpeg_m2m_ops = {
 	.device_run	= exynos4_jpeg_device_run,
 	.job_ready	= s5p_jpeg_job_ready,
 	.job_abort	= s5p_jpeg_job_abort,
@@ -1849,7 +1849,6 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 {
 	struct s5p_jpeg *jpeg;
 	struct resource *res;
-	struct v4l2_m2m_ops *samsung_jpeg_m2m_ops;
 	int ret;
 
 	if (!pdev->dev.of_node)
@@ -1903,13 +1902,8 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 		goto clk_get_rollback;
 	}
 
-	if (jpeg->variant->version == SJPEG_S5P)
-		samsung_jpeg_m2m_ops = &s5p_jpeg_m2m_ops;
-	else
-		samsung_jpeg_m2m_ops = &exynos_jpeg_m2m_ops;
-
 	/* mem2mem device */
-	jpeg->m2m_dev = v4l2_m2m_init(samsung_jpeg_m2m_ops);
+	jpeg->m2m_dev = v4l2_m2m_init(jpeg->variant->m2m_ops);
 	if (IS_ERR(jpeg->m2m_dev)) {
 		v4l2_err(&jpeg->v4l2_dev, "Failed to init mem2mem device\n");
 		ret = PTR_ERR(jpeg->m2m_dev);
@@ -2098,12 +2092,14 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
 static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
 	.version	= SJPEG_S5P,
 	.jpeg_irq	= s5p_jpeg_irq,
+	.m2m_ops	= &s5p_jpeg_m2m_ops,
 	.fmt_ver_flag	= SJPEG_FMT_FLAG_S5P,
 };
 
 static struct s5p_jpeg_variant exynos4_jpeg_drvdata = {
 	.version	= SJPEG_EXYNOS4,
 	.jpeg_irq	= exynos4_jpeg_irq,
+	.m2m_ops	= &exynos4_jpeg_m2m_ops,
 	.fmt_ver_flag	= SJPEG_FMT_FLAG_EXYNOS4,
 };
 
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.h b/drivers/media/platform/s5p-jpeg/jpeg-core.h
index c222436..3e47863 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.h
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.h
@@ -117,9 +117,10 @@ struct s5p_jpeg {
 };
 
 struct s5p_jpeg_variant {
-	unsigned int	version;
-	unsigned int	fmt_ver_flag;
-	irqreturn_t	(*jpeg_irq)(int irq, void *priv);
+	unsigned int		version;
+	unsigned int		fmt_ver_flag;
+	struct v4l2_m2m_ops	*m2m_ops;
+	irqreturn_t		(*jpeg_irq)(int irq, void *priv);
 };
 
 /**
-- 
1.7.9.5

[PATCH 4/8] [media] s5p-jpeg: Fix build break when CONFIG_OF is undefined

[Jacek Anaszewski]

This patch fixes build break occurring when
there is no support for Device Tree turned on
in the kernel configuration. In such a case only
the driver variant for S5PC210 SoC will be available.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 4f4dc81..913a027 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1837,7 +1837,7 @@ static irqreturn_t exynos4_jpeg_irq(int irq, void *priv)
 	return IRQ_HANDLED;
 }
 
-static void *jpeg_get_drv_data(struct platform_device *pdev);
+static void *jpeg_get_drv_data(struct device *dev);
 
 /*
  * ============================================================================
@@ -1851,15 +1851,12 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret;
 
-	if (!pdev->dev.of_node)
-		return -ENODEV;
-
 	/* JPEG IP abstraction struct */
 	jpeg = devm_kzalloc(&pdev->dev, sizeof(struct s5p_jpeg), GFP_KERNEL);
 	if (!jpeg)
 		return -ENOMEM;
 
-	jpeg->variant = jpeg_get_drv_data(pdev);
+	jpeg->variant = jpeg_get_drv_data(&pdev->dev);
 
 	mutex_init(&jpeg->lock);
 	spin_lock_init(&jpeg->slock);
@@ -2088,7 +2085,6 @@ static const struct dev_pm_ops s5p_jpeg_pm_ops = {
 	SET_RUNTIME_PM_OPS(s5p_jpeg_runtime_suspend, s5p_jpeg_runtime_resume, NULL)
 };
 
-#ifdef CONFIG_OF
 static struct s5p_jpeg_variant s5p_jpeg_drvdata = {
 	.version	= SJPEG_S5P,
 	.jpeg_irq	= s5p_jpeg_irq,
@@ -2119,19 +2115,21 @@ static const struct of_device_id samsung_jpeg_match[] = {
 
 MODULE_DEVICE_TABLE(of, samsung_jpeg_match);
 
-static void *jpeg_get_drv_data(struct platform_device *pdev)
+static void *jpeg_get_drv_data(struct device *dev)
 {
 	struct s5p_jpeg_variant *driver_data = NULL;
 	const struct of_device_id *match;
 
+	if (!IS_ENABLED(CONFIG_OF) || dev->of_node == NULL)
+		return &s5p_jpeg_drvdata;
+
 	match = of_match_node(of_match_ptr(samsung_jpeg_match),
-					 pdev->dev.of_node);
+					dev->of_node);
 	if (match)
 		driver_data = (struct s5p_jpeg_variant *)match->data;
 
 	return driver_data;
 }
-#endif
 
 static struct platform_driver s5p_jpeg_driver = {
 	.probe = s5p_jpeg_probe,
-- 
1.7.9.5

[PATCH 5/8] [media] s5p-jpeg: g_selection callback should always succeed

[Jacek Anaszewski]

Remove erroneous guard preventing successful execution of
g_selection callback in case the driver variant is different
from SJPEG_S5P.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 913a027..8a15c4a 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1177,8 +1177,7 @@ static int s5p_jpeg_g_selection(struct file *file, void *priv,
 	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
 
 	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
-	    s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-	    ctx->jpeg->variant->version != SJPEG_S5P)
+	    s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
 	/* For JPEG blob active == default == bounds */
-- 
1.7.9.5

[PATCH 6/8] [media] s5p-jpeg: Fix sysmmu page fault

[Jacek Anaszewski]

This patch fixes jpeg sysmmu page fault on Exynos4x12 SoCs.
During encoding Exynos4x12 SoCs access wider memory area
than it results from Image_x and Image_y values written to
the JPEG_IMAGE_SIZE register. In order to avoid sysmmu page
fault apply proper output buffer size alignment.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |   46 +++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 8a15c4a..d266e78 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1106,6 +1106,32 @@ static int s5p_jpeg_try_fmt_vid_out(struct file *file, void *priv,
 	return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_OUTPUT);
 }
 
+static int exynos4_jpeg_get_output_buffer_size(struct s5p_jpeg_ctx *ctx,
+						struct v4l2_format *f,
+						int fmt_depth)
+{
+	struct v4l2_pix_format *pix = &f->fmt.pix;
+	u32 pix_fmt = f->fmt.pix.pixelformat;
+	int w = pix->width, h = pix->height, wh_align;
+
+	if (pix_fmt == V4L2_PIX_FMT_RGB32 ||
+	    pix_fmt == V4L2_PIX_FMT_NV24 ||
+	    pix_fmt == V4L2_PIX_FMT_NV42 ||
+	    pix_fmt == V4L2_PIX_FMT_NV12 ||
+	    pix_fmt == V4L2_PIX_FMT_NV21 ||
+	    pix_fmt == V4L2_PIX_FMT_YUV420)
+		wh_align = 4;
+	else
+		wh_align = 1;
+
+	jpeg_bound_align_image(&w, S5P_JPEG_MIN_WIDTH,
+			       S5P_JPEG_MAX_WIDTH, wh_align,
+			       &h, S5P_JPEG_MIN_HEIGHT,
+			       S5P_JPEG_MAX_HEIGHT, wh_align);
+
+	return w * h * fmt_depth >> 3;
+}
+
 static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct v4l2_format *f)
 {
 	struct vb2_queue *vq;
@@ -1132,10 +1158,24 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct v4l2_format *f)
 	q_data->fmt = s5p_jpeg_find_format(ct, pix->pixelformat, f_type);
 	q_data->w = pix->width;
 	q_data->h = pix->height;
-	if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG)
-		q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
-	else
+	if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG) {
+		/*
+		 * During encoding Exynos4x12 SoCs access wider memory area
+		 * than it results from Image_x and Image_y values written to
+		 * the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
+		 * page fault calculate proper buffer size in such a case.
+		 */
+		if (ct->jpeg->variant->version == SJPEG_EXYNOS4 &&
+		    f_type == FMT_TYPE_OUTPUT && ct->mode == S5P_JPEG_ENCODE)
+			q_data->size = exynos4_jpeg_get_output_buffer_size(ct,
+							f,
+							q_data->fmt->depth);
+		else
+			q_data->size = q_data->w * q_data->h *
+						q_data->fmt->depth >> 3;
+	} else {
 		q_data->size = pix->sizeimage;
+	}
 
 	if (f_type == FMT_TYPE_OUTPUT) {
 		ctrl_subs = v4l2_ctrl_find(&ct->ctrl_handler,
-- 
1.7.9.5

[PATCH 7/8] [media] s5p_jpeg: Prevent JPEG 4:2:0 > YUV 4:2:0 decompression

[Jacek Anaszewski]

Prevent decompression of a JPEG 4:2:0 with odd width to
the YUV 4:2:0 compliant formats for Exynos4x12 SoCs and
adjust capture format to RGB565 in such a case. This is
required because the configuration would produce a raw
image with broken luma component.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |   24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index d266e78..9228bcb 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1064,15 +1064,17 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file, void *priv,
 		return -EINVAL;
 	}
 
+	if ((ctx->jpeg->variant->version != SJPEG_EXYNOS4) ||
+	    (ctx->mode != S5P_JPEG_DECODE))
+		goto exit;
+
 	/*
 	 * The exynos4x12 device requires resulting YUV image
 	 * subsampling not to be lower than the input jpeg subsampling.
 	 * If this requirement is not met then downgrade the requested
 	 * capture format to the one with subsampling equal to the input jpeg.
 	 */
-	if ((ctx->jpeg->variant->version == SJPEG_EXYNOS4) &&
-	    (ctx->mode == S5P_JPEG_DECODE) &&
-	    (fmt->flags & SJPEG_FMT_NON_RGB) &&
+	if ((fmt->flags & SJPEG_FMT_NON_RGB) &&
 	    (fmt->subsampling < ctx->subsampling)) {
 		ret = s5p_jpeg_adjust_fourcc_to_subsampling(ctx->subsampling,
 							    fmt->fourcc,
@@ -1085,6 +1087,22 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file, void *priv,
 							FMT_TYPE_CAPTURE);
 	}
 
+	if (ctx->subsampling == V4L2_JPEG_CHROMA_SUBSAMPLING_420 &&
+	    (ctx->out_q.w & 1) &&
+	    (pix->pixelformat == V4L2_PIX_FMT_NV12 ||
+	     pix->pixelformat == V4L2_PIX_FMT_NV21 ||
+	     pix->pixelformat == V4L2_PIX_FMT_YUV420)) {
+		pix->pixelformat = V4L2_PIX_FMT_RGB565;
+		fmt = s5p_jpeg_find_format(ctx, pix->pixelformat,
+							FMT_TYPE_CAPTURE);
+		v4l2_info(&ctx->jpeg->v4l2_dev,
+			  "Adjusted capture fourcc to RGB565. Decompression\n"
+			  "of a JPEG file with 4:2:0 subsampling and odd\n"
+			  "width to the YUV 4:2:0 compliant formats produces\n"
+			  "a raw image with broken luma component.\n");
+	}
+
+exit:
 	return vidioc_try_fmt(f, fmt, ctx, FMT_TYPE_CAPTURE);
 }
 
-- 
1.7.9.5

[PATCH 8/8] [media] s5p_jpeg: Fix NV12 format entry related to S5C2120 SoC

[Jacek Anaszewski]

S5PC210 SoC doesn't support encoding NV12 raw images. Remove
the relavant flag from the respective entry in the sjpeg_formats
array.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 9228bcb..a57a1e0 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -192,8 +192,7 @@ static struct s5p_jpeg_fmt sjpeg_formats[] = {
 		.colplanes	= 2,
 		.h_align	= 4,
 		.v_align	= 4,
-		.flags		= SJPEG_FMT_FLAG_ENC_OUTPUT |
-				  SJPEG_FMT_FLAG_DEC_CAPTURE |
+		.flags		= SJPEG_FMT_FLAG_DEC_CAPTURE |
 				  SJPEG_FMT_FLAG_S5P |
 				  SJPEG_FMT_NON_RGB,
 		.subsampling	= V4L2_JPEG_CHROMA_SUBSAMPLING_420,
-- 
1.7.9.5

Re: [PATCH 4/8] [media] s5p-jpeg: Fix build break when CONFIG_OF is undefined

[Sachin Kamat]

Hi Jacek,

On 7 April 2014 18:46, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> This patch fixes build break occurring when
> there is no support for Device Tree turned on
> in the kernel configuration. In such a case only
> the driver variant for S5PC210 SoC will be available.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
<snip>

>
> +       if (!IS_ENABLED(CONFIG_OF) || dev->of_node == NULL)

!dev->of_node instead of equating to NULL.


> +               return &s5p_jpeg_drvdata;
> +
>         match = of_match_node(of_match_ptr(samsung_jpeg_match),

Since you are returning above if CONFIG_OF is not enabled, of_match_ptr
is not needed.

-- 
With warm regards,
Sachin

Re: [PATCH 7/8] [media] s5p_jpeg: Prevent JPEG 4:2:0 > YUV 4:2:0 decompression

[Sachin Kamat]

Hi Jacek,

On 7 April 2014 18:46, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> Prevent decompression of a JPEG 4:2:0 with odd width to
> the YUV 4:2:0 compliant formats for Exynos4x12 SoCs and
> adjust capture format to RGB565 in such a case. This is
> required because the configuration would produce a raw
> image with broken luma component.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
<snip>

> +       if (ctx->subsampling == V4L2_JPEG_CHROMA_SUBSAMPLING_420 &&
> +           (ctx->out_q.w & 1) &&
> +           (pix->pixelformat == V4L2_PIX_FMT_NV12 ||
> +            pix->pixelformat == V4L2_PIX_FMT_NV21 ||
> +            pix->pixelformat == V4L2_PIX_FMT_YUV420)) {
> +               pix->pixelformat = V4L2_PIX_FMT_RGB565;
> +               fmt = s5p_jpeg_find_format(ctx, pix->pixelformat,
> +                                                       FMT_TYPE_CAPTURE);
> +               v4l2_info(&ctx->jpeg->v4l2_dev,
> +                         "Adjusted capture fourcc to RGB565. Decompression\n"
> +                         "of a JPEG file with 4:2:0 subsampling and odd\n"
> +                         "width to the YUV 4:2:0 compliant formats produces\n"
> +                         "a raw image with broken luma component.\n");

This could be made a comment in the code rather than a info message.

-- 
With warm regards,
Sachin

Re: [PATCH 7/8] [media] s5p_jpeg: Prevent JPEG 4:2:0 > YUV 4:2:0 decompression

[Jacek Anaszewski]

On 04/08/2014 09:49 AM, Sachin Kamat wrote:
> Hi Jacek,
>
> On 7 April 2014 18:46, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
>> Prevent decompression of a JPEG 4:2:0 with odd width to
>> the YUV 4:2:0 compliant formats for Exynos4x12 SoCs and
>> adjust capture format to RGB565 in such a case. This is
>> required because the configuration would produce a raw
>> image with broken luma component.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
> <snip>
>
>> +       if (ctx->subsampling == V4L2_JPEG_CHROMA_SUBSAMPLING_420 &&
>> +           (ctx->out_q.w & 1) &&
>> +           (pix->pixelformat == V4L2_PIX_FMT_NV12 ||
>> +            pix->pixelformat == V4L2_PIX_FMT_NV21 ||
>> +            pix->pixelformat == V4L2_PIX_FMT_YUV420)) {
>> +               pix->pixelformat = V4L2_PIX_FMT_RGB565;
>> +               fmt = s5p_jpeg_find_format(ctx, pix->pixelformat,
>> +                                                       FMT_TYPE_CAPTURE);
>> +               v4l2_info(&ctx->jpeg->v4l2_dev,
>> +                         "Adjusted capture fourcc to RGB565. Decompression\n"
>> +                         "of a JPEG file with 4:2:0 subsampling and odd\n"
>> +                         "width to the YUV 4:2:0 compliant formats produces\n"
>> +                         "a raw image with broken luma component.\n");
>
> This could be made a comment in the code rather than a info message.
>

Hello Sachin,

Thanks for the review. I put it into info message because this is
rather hard for the user to figure out why the adjustment occurred,
bearing in mind that JPEG with the same subsampling and even width
is decompressed properly. This is not a common adjustment like
alignment, and thus in my opinion it requires displaying the
information. Are there some rules that say what cases are relevant
for using the v4l2_info macro?

Regards,
Jacek Anaszewski

Re: [PATCH 7/8] [media] s5p_jpeg: Prevent JPEG 4:2:0 > YUV 4:2:0 decompression

[Sachin Kamat]

Hi Jacek,

On 9 April 2014 13:01, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
> On 04/08/2014 09:49 AM, Sachin Kamat wrote:
>>

> Hello Sachin,
>
> Thanks for the review. I put it into info message because this is
> rather hard for the user to figure out why the adjustment occurred,
> bearing in mind that JPEG with the same subsampling and even width
> is decompressed properly. This is not a common adjustment like
> alignment, and thus in my opinion it requires displaying the
> information. Are there some rules that say what cases are relevant
> for using the v4l2_info macro?

Not really, but generally info messages are concise and detailed explanations
provided as part of comments.

-- 
With warm regards,
Sachin

Re: [PATCH 7/8] [media] s5p_jpeg: Prevent JPEG 4:2:0 > YUV 4:2:0 decompression

[Jacek Anaszewski]

On 04/09/2014 09:56 AM, Sachin Kamat wrote:
> Hi Jacek,
>
> On 9 April 2014 13:01, Jacek Anaszewski <j.anaszewski@samsung.com> wrote:
>> On 04/08/2014 09:49 AM, Sachin Kamat wrote:
>>>
>
>> Hello Sachin,
>>
>> Thanks for the review. I put it into info message because this is
>> rather hard for the user to figure out why the adjustment occurred,
>> bearing in mind that JPEG with the same subsampling and even width
>> is decompressed properly. This is not a common adjustment like
>> alignment, and thus in my opinion it requires displaying the
>> information. Are there some rules that say what cases are relevant
>> for using the v4l2_info macro?
>
> Not really, but generally info messages are concise and detailed explanations
> provided as part of comments.
>

Thanks for the explanation, I will stick to it.

Regards,
Jacek Anaszewski