Re: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M-Scaler
On 08/19/2013 12:58 PM, Shaik Ameer Basha wrote: This patch adds support for M-Scaler (M2M Scaler) device which is a new device for scaling, blending, color fill and color space conversion on EXYNOS5 SoCs. This device supports the followings as key feature. input image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB, L8A8 and L8 output image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB input rotation - 0/90/180/270 degree, X/Y/XY Flip scale ratio - 1/4 scale down to 16 scale up color space conversion - RGB to YUV / YUV to RGB Size - Input : 16x16 to 8192x8192 - Output: 4x4 to 8192x8192 alpha blending, color fill We don't have good support for alpha blending in v4l2. For example, the s5p-g2d v4l2 driver only supports a subset of features of the device. The G2D IP is probably better utilized through the exynos drm driver. It might be a matter of designing proper controls though for such devices that generate image by blending data from the output and capture buffers. Signed-off-by: Shaik Ameer Bashashaik.am...@samsung.com --- drivers/media/platform/exynos-mscl/mscl-regs.c | 318 drivers/media/platform/exynos-mscl/mscl-regs.h | 282 + Shouldn't we call this driver exynos5-scaler and this files would be renamed to drivers/media/platform/exynos5-scaler/scaler-regs.[ch]. Or maybe put maybe put the SCALER, G-SCALER drivers into common exynos5-is directory ? I don't have strong preference. But we planned gscaler to be moved into the exynos5-is directory. 2 files changed, 600 insertions(+) create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.c create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.h diff --git a/drivers/media/platform/exynos-mscl/mscl-regs.c b/drivers/media/platform/exynos-mscl/mscl-regs.c new file mode 100644 index 000..9354afc --- /dev/null +++ b/drivers/media/platform/exynos-mscl/mscl-regs.c @@ -0,0 +1,318 @@ +/* + * Copyright (c) 2013 - 2014 Samsung Electronics Co., Ltd. Just make it 2013 ? I think we can manage to merge this diver this year :) + * http://www.samsung.com + * + * Samsung EXYNOS5 SoC series M-Scaler driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. Are you sure you want to have this or (at your option) any later version clause ? + */ + +#includelinux/delay.h +#includelinux/platform_device.h + +#include mscl-core.h How about defining register access helpers like: u32 mscl_read(struct mscl_dev *dev, u32 offset) { readl(dev-regs + offset); } void mscl_write(struct mscl_dev *dev, u32 offset, u32 value) { writel(value, dev-regs + offset); } perhaps as static inline in mscl-regs.h ? +void mscl_hw_set_sw_reset(struct mscl_dev *dev) +{ + u32 cfg; + + cfg = readl(dev-regs + MSCL_CFG); + cfg |= MSCL_CFG_SOFT_RESET; + + writel(cfg, dev-regs + MSCL_CFG); +} + +int mscl_wait_reset(struct mscl_dev *dev) +{ + unsigned long end = jiffies + msecs_to_jiffies(50); Don't you want a #define for this timeout ? + u32 cfg, reset_done = 0; + + while (time_before(jiffies, end)) { + cfg = readl(dev-regs + MSCL_CFG); + if (!(cfg MSCL_CFG_SOFT_RESET)) { + reset_done = 1; + break; + } + usleep_range(10, 20); + } + + /* write any value to r/w reg and read it back */ + while (reset_done) { + + /* [TBD] need to define number of tries before returning +* -EBUSY to the caller +*/ CodingStyle: wrong multi-line comment style + writel(MSCL_CFG_SOFT_RESET_CHECK_VAL, + dev-regs + MSCL_CFG_SOFT_RESET_CHECK_REG); + if (MSCL_CFG_SOFT_RESET_CHECK_VAL == + readl(dev-regs + MSCL_CFG_SOFT_RESET_CHECK_REG)) + return 0; + } + + return -EBUSY; +} + +void mscl_hw_set_irq_mask(struct mscl_dev *dev, int interrupt, bool mask) +{ + u32 cfg; + + switch (interrupt) { + case MSCL_INT_TIMEOUT: + case MSCL_INT_ILLEGAL_BLEND: + case MSCL_INT_ILLEGAL_RATIO: + case MSCL_INT_ILLEGAL_DST_HEIGHT: + case MSCL_INT_ILLEGAL_DST_WIDTH: + case MSCL_INT_ILLEGAL_DST_V_POS: + case
Re: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M-Scaler
Hi Inki Dae, Thanks for the review. On Mon, Aug 19, 2013 at 6:18 PM, Inki Dae inki@samsung.com wrote: Just quick review. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Shaik Ameer Basha Sent: Monday, August 19, 2013 7:59 PM To: linux-media@vger.kernel.org; linux-samsung-...@vger.kernel.org Cc: s.nawro...@samsung.com; posc...@google.com; arun...@samsung.com; shaik.am...@samsung.com Subject: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M-Scaler This patch adds support for M-Scaler (M2M Scaler) device which is a new device for scaling, blending, color fill and color space conversion on EXYNOS5 SoCs. This device supports the followings as key feature. input image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB, L8A8 and L8 output image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB input rotation - 0/90/180/270 degree, X/Y/XY Flip scale ratio - 1/4 scale down to 16 scale up color space conversion - RGB to YUV / YUV to RGB Size - Input : 16x16 to 8192x8192 - Output: 4x4 to 8192x8192 alpha blending, color fill Signed-off-by: Shaik Ameer Basha shaik.am...@samsung.com --- drivers/media/platform/exynos-mscl/mscl-regs.c | 318 drivers/media/platform/exynos-mscl/mscl-regs.h | 282 + 2 files changed, 600 insertions(+) create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.c create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.h diff --git a/drivers/media/platform/exynos-mscl/mscl-regs.c b/drivers/media/platform/exynos-mscl/mscl-regs.c new file mode 100644 index 000..9354afc --- /dev/null +++ b/drivers/media/platform/exynos-mscl/mscl-regs.c @@ -0,0 +1,318 @@ +/* + * Copyright (c) 2013 - 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung EXYNOS5 SoC series M-Scaler driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + */ + +#include linux/delay.h +#include linux/platform_device.h + +#include mscl-core.h + +void mscl_hw_set_sw_reset(struct mscl_dev *dev) +{ + u32 cfg; + + cfg = readl(dev-regs + MSCL_CFG); + cfg |= MSCL_CFG_SOFT_RESET; + + writel(cfg, dev-regs + MSCL_CFG); +} + +int mscl_wait_reset(struct mscl_dev *dev) +{ + unsigned long end = jiffies + msecs_to_jiffies(50); What does 50 mean? + u32 cfg, reset_done = 0; + Please describe why the below codes are needed. As per the Documentation, SOFT RESET: Writing 1 to this bit generates software reset. To check the completion of the reset, wait until this field becomes zero, then wrie an arbitrary value to any of RW registers and read it. If the read data matches the written data, it means SW reset succeeded. Otherwise, repeat write read until matched. Thie below code tries to do the same (as per user manual). and in the above msec_to_jiffies(50), 50 is the 50msec wait. before checking the SOFT RESET is really done. Is it good to ignore this checks? + while (time_before(jiffies, end)) { + cfg = readl(dev-regs + MSCL_CFG); + if (!(cfg MSCL_CFG_SOFT_RESET)) { + reset_done = 1; + break; + } + usleep_range(10, 20); + } + + /* write any value to r/w reg and read it back */ + while (reset_done) { + + /* [TBD] need to define number of tries before returning + * -EBUSY to the caller + */ + + writel(MSCL_CFG_SOFT_RESET_CHECK_VAL, + dev-regs + MSCL_CFG_SOFT_RESET_CHECK_REG); + if (MSCL_CFG_SOFT_RESET_CHECK_VAL == + readl(dev-regs + MSCL_CFG_SOFT_RESET_CHECK_REG)) + return 0; + } + + return -EBUSY; +} + +void mscl_hw_set_irq_mask(struct mscl_dev *dev, int interrupt, bool mask) +{ + u32 cfg; + + switch (interrupt) { + case MSCL_INT_TIMEOUT: + case MSCL_INT_ILLEGAL_BLEND: + case MSCL_INT_ILLEGAL_RATIO: + case MSCL_INT_ILLEGAL_DST_HEIGHT: + case MSCL_INT_ILLEGAL_DST_WIDTH: + case MSCL_INT_ILLEGAL_DST_V_POS: + case MSCL_INT_ILLEGAL_DST_H_POS: + case MSCL_INT_ILLEGAL_DST_C_SPAN: + case MSCL_INT_ILLEGAL_DST_Y_SPAN: +
RE: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M-Scaler
-Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Shaik Ameer Basha Sent: Tuesday, August 20, 2013 5:07 PM To: Inki Dae Cc: Shaik Ameer Basha; LMML; linux-samsung-...@vger.kernel.org; c...@samsung.com; Sylwester Nawrocki; posc...@google.com; Arun Kumar K Subject: Re: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M- Scaler Hi Inki Dae, Thanks for the review. On Mon, Aug 19, 2013 at 6:18 PM, Inki Dae inki@samsung.com wrote: Just quick review. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Shaik Ameer Basha Sent: Monday, August 19, 2013 7:59 PM To: linux-media@vger.kernel.org; linux-samsung-...@vger.kernel.org Cc: s.nawro...@samsung.com; posc...@google.com; arun...@samsung.com; shaik.am...@samsung.com Subject: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M- Scaler This patch adds support for M-Scaler (M2M Scaler) device which is a new device for scaling, blending, color fill and color space conversion on EXYNOS5 SoCs. This device supports the followings as key feature. input image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB, L8A8 and L8 output image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB input rotation - 0/90/180/270 degree, X/Y/XY Flip scale ratio - 1/4 scale down to 16 scale up color space conversion - RGB to YUV / YUV to RGB Size - Input : 16x16 to 8192x8192 - Output: 4x4 to 8192x8192 alpha blending, color fill Signed-off-by: Shaik Ameer Basha shaik.am...@samsung.com --- drivers/media/platform/exynos-mscl/mscl-regs.c | 318 drivers/media/platform/exynos-mscl/mscl-regs.h | 282 + 2 files changed, 600 insertions(+) create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.c create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.h diff --git a/drivers/media/platform/exynos-mscl/mscl-regs.c b/drivers/media/platform/exynos-mscl/mscl-regs.c new file mode 100644 index 000..9354afc --- /dev/null +++ b/drivers/media/platform/exynos-mscl/mscl-regs.c @@ -0,0 +1,318 @@ +/* + * Copyright (c) 2013 - 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung EXYNOS5 SoC series M-Scaler driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + */ + +#include linux/delay.h +#include linux/platform_device.h + +#include mscl-core.h + +void mscl_hw_set_sw_reset(struct mscl_dev *dev) +{ + u32 cfg; + + cfg = readl(dev-regs + MSCL_CFG); + cfg |= MSCL_CFG_SOFT_RESET; + + writel(cfg, dev-regs + MSCL_CFG); +} + +int mscl_wait_reset(struct mscl_dev *dev) +{ + unsigned long end = jiffies + msecs_to_jiffies(50); What does 50 mean? + u32 cfg, reset_done = 0; + Please describe why the below codes are needed. As per the Documentation, SOFT RESET: Writing 1 to this bit generates software reset. To check the completion of the reset, wait until this field becomes zero, then wrie an arbitrary value to any of RW registers and read it. If the read data matches the written data, it means SW reset succeeded. Otherwise, repeat write read until matched. Thie below code tries to do the same (as per user manual). and in the above msec_to_jiffies(50), 50 is the 50msec wait. before checking the SOFT RESET is really done. Is it good to ignore this checks? No, I mean that someone may want to understand your codes so leave comments enough for them. Thanks, Inki Dae + while (time_before(jiffies, end)) { + cfg = readl(dev-regs + MSCL_CFG); + if (!(cfg MSCL_CFG_SOFT_RESET)) { + reset_done = 1; + break; + } + usleep_range(10, 20); + } + + /* write any value to r/w reg and read it back */ + while (reset_done) { + + /* [TBD] need to define number of tries before returning + * -EBUSY to the caller + */ + + writel(MSCL_CFG_SOFT_RESET_CHECK_VAL, + dev-regs + MSCL_CFG_SOFT_RESET_CHECK_REG
Re: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M-Scaler
On Tue, Aug 20, 2013 at 2:13 PM, Inki Dae inki@samsung.com wrote: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Shaik Ameer Basha Sent: Tuesday, August 20, 2013 5:07 PM To: Inki Dae Cc: Shaik Ameer Basha; LMML; linux-samsung-...@vger.kernel.org; c...@samsung.com; Sylwester Nawrocki; posc...@google.com; Arun Kumar K Subject: Re: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M- Scaler Hi Inki Dae, Thanks for the review. On Mon, Aug 19, 2013 at 6:18 PM, Inki Dae inki@samsung.com wrote: Just quick review. -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Shaik Ameer Basha Sent: Monday, August 19, 2013 7:59 PM To: linux-media@vger.kernel.org; linux-samsung-...@vger.kernel.org Cc: s.nawro...@samsung.com; posc...@google.com; arun...@samsung.com; shaik.am...@samsung.com Subject: [PATCH v2 1/5] [media] exynos-mscl: Add new driver for M- Scaler This patch adds support for M-Scaler (M2M Scaler) device which is a new device for scaling, blending, color fill and color space conversion on EXYNOS5 SoCs. This device supports the followings as key feature. input image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB, L8A8 and L8 output image format - YCbCr420 2P(UV/VU), 3P - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P - YCbCr444 2P(UV,VU), 3P - RGB565, ARGB1555, ARGB, ARGB, RGBA - Pre-multiplexed ARGB input rotation - 0/90/180/270 degree, X/Y/XY Flip scale ratio - 1/4 scale down to 16 scale up color space conversion - RGB to YUV / YUV to RGB Size - Input : 16x16 to 8192x8192 - Output: 4x4 to 8192x8192 alpha blending, color fill Signed-off-by: Shaik Ameer Basha shaik.am...@samsung.com --- drivers/media/platform/exynos-mscl/mscl-regs.c | 318 drivers/media/platform/exynos-mscl/mscl-regs.h | 282 + 2 files changed, 600 insertions(+) create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.c create mode 100644 drivers/media/platform/exynos-mscl/mscl-regs.h diff --git a/drivers/media/platform/exynos-mscl/mscl-regs.c b/drivers/media/platform/exynos-mscl/mscl-regs.c new file mode 100644 index 000..9354afc --- /dev/null +++ b/drivers/media/platform/exynos-mscl/mscl-regs.c @@ -0,0 +1,318 @@ +/* + * Copyright (c) 2013 - 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung EXYNOS5 SoC series M-Scaler driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation, either version 2 of the License, + * or (at your option) any later version. + */ + +#include linux/delay.h +#include linux/platform_device.h + +#include mscl-core.h + +void mscl_hw_set_sw_reset(struct mscl_dev *dev) +{ + u32 cfg; + + cfg = readl(dev-regs + MSCL_CFG); + cfg |= MSCL_CFG_SOFT_RESET; + + writel(cfg, dev-regs + MSCL_CFG); +} + +int mscl_wait_reset(struct mscl_dev *dev) +{ + unsigned long end = jiffies + msecs_to_jiffies(50); What does 50 mean? + u32 cfg, reset_done = 0; + Please describe why the below codes are needed. As per the Documentation, SOFT RESET: Writing 1 to this bit generates software reset. To check the completion of the reset, wait until this field becomes zero, then wrie an arbitrary value to any of RW registers and read it. If the read data matches the written data, it means SW reset succeeded. Otherwise, repeat write read until matched. Thie below code tries to do the same (as per user manual). and in the above msec_to_jiffies(50), 50 is the 50msec wait. before checking the SOFT RESET is really done. Is it good to ignore this checks? No, I mean that someone may want to understand your codes so leave comments enough for them. Ok. thanks. I will add more comments. :) Regards, Shaik Ameer Basha Thanks, Inki Dae + while (time_before(jiffies, end)) { + cfg = readl(dev-regs + MSCL_CFG); + if (!(cfg MSCL_CFG_SOFT_RESET)) { + reset_done = 1; + break; + } + usleep_range(10, 20); + } + + /* write any value to r/w reg and read it back */ + while (reset_done) { + + /* [TBD] need to define number of tries before returning + * -EBUSY to the caller