* [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format
@ 2015-09-22 5:14 Josh Wu
2015-09-22 5:14 ` [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs Josh Wu
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Josh Wu @ 2015-09-22 5:14 UTC (permalink / raw)
To: Linux Media Mailing List, linux-arm-kernel, Guennadi Liakhovetski
Cc: Laurent Pinchart, Josh Wu
This series will enable preview path support in atmel-isi. Which can
make atmel-isi convert the YUV format (from sensor) to RGB565 format.
Josh Wu (5):
media: atmel-isi: correct yuv swap according to different sensor
outputs
media: atmel-isi: prepare for the support of preview path
media: atmel-isi: add code to setup correct resolution for preview
path
media: atmel-isi: setup YCC_SWAP correctly when using preview path
media: atmel-isi: support RGB565 output when sensor output YUV formats
drivers/media/platform/soc_camera/atmel-isi.c | 181 ++++++++++++++++++++------
drivers/media/platform/soc_camera/atmel-isi.h | 10 ++
2 files changed, 148 insertions(+), 43 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-09-22 5:14 [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format Josh Wu
@ 2015-09-22 5:14 ` Josh Wu
2015-10-04 16:43 ` Guennadi Liakhovetski
2015-09-22 5:14 ` [PATCH 2/5] media: atmel-isi: prepare for the support of preview path Josh Wu
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Josh Wu @ 2015-09-22 5:14 UTC (permalink / raw)
To: Linux Media Mailing List, linux-arm-kernel, Guennadi Liakhovetski
Cc: Laurent Pinchart, Josh Wu
we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
sensor output and Atmel ISI output format.
Current there are two cases Atmel ISI supported:
1. Atmel ISI outputs YUYV format.
This case we need to setup YCC_SWAP according to sensor output
format.
2. Atmel ISI output a pass-through formats, which means no swap.
Just setup YCC_SWAP as default with no swap.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 43 ++++++++++++++++++++-------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 45e304a..df64294 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
}
+static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
+ const struct soc_camera_format_xlate *xlate)
+{
+ /* By default, no swap for the codec path of Atmel ISI. So codec
+ * output is same as sensor's output.
+ * For instance, if sensor's output is YUYV, then codec outputs YUYV.
+ * And if sensor's output is UYVY, then codec outputs UYVY.
+ */
+ u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
+
+ if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
+ /* all convert to YUYV */
+ switch (xlate->code) {
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
+ break;
+ case MEDIA_BUS_FMT_UYVY8_2X8:
+ cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
+ break;
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
+ break;
+ }
+ }
+
+ return cfg2_yuv_swap;
+}
+
static void configure_geometry(struct atmel_isi *isi, u32 width,
- u32 height, u32 code)
+ u32 height, const struct soc_camera_format_xlate *xlate)
{
u32 cfg2;
/* According to sensor's output format to set cfg2 */
- switch (code) {
+ switch (xlate->code) {
default:
/* Grey */
case MEDIA_BUS_FMT_Y8_1X8:
@@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi *isi, u32 width,
break;
/* YUV */
case MEDIA_BUS_FMT_VYUY8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_UYVY8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_YVYU8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
- break;
case MEDIA_BUS_FMT_YUYV8_2X8:
- cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
+ cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
+ setup_cfg2_yuv_swap(isi, xlate);
break;
/* RGB, TODO */
}
@@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
isi_writel(isi, ISI_INTDIS, (u32)~0UL);
configure_geometry(isi, icd->user_width, icd->user_height,
- icd->current_fmt->code);
+ icd->current_fmt);
spin_lock_irq(&isi->lock);
/* Clear any pending interrupt */
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] media: atmel-isi: prepare for the support of preview path
2015-09-22 5:14 [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format Josh Wu
2015-09-22 5:14 ` [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs Josh Wu
@ 2015-09-22 5:14 ` Josh Wu
2015-09-22 5:14 ` [PATCH 3/5] media: atmel-isi: add code to setup correct resolution for " Josh Wu
` (2 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Josh Wu @ 2015-09-22 5:14 UTC (permalink / raw)
To: Linux Media Mailing List, linux-arm-kernel, Guennadi Liakhovetski
Cc: Laurent Pinchart, Josh Wu
Atmel ISI support a preview path which can output RGB data.
So this patch introduces a bool variable to choose which path is
enabled currently. And also we need setup corresponding path registers.
By default the preview path is disabled. We only use Codec path.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 72 ++++++++++++++++++---------
1 file changed, 49 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index df64294..e6f4ade 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -79,6 +79,7 @@ struct atmel_isi {
dma_addr_t fb_descriptors_phys;
struct list_head dma_desc_head;
struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
+ bool enable_preview_path;
struct completion complete;
/* ISI peripherial clock */
@@ -199,11 +200,19 @@ static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
/* start next dma frame. */
isi->active = list_entry(isi->video_buffer_list.next,
struct frame_buffer, list);
- isi_writel(isi, ISI_DMA_C_DSCR,
- (u32)isi->active->p_dma_desc->fbd_phys);
- isi_writel(isi, ISI_DMA_C_CTRL,
- ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
- isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ if (!isi->enable_preview_path) {
+ isi_writel(isi, ISI_DMA_C_DSCR,
+ (u32)isi->active->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_C_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ } else {
+ isi_writel(isi, ISI_DMA_P_DSCR,
+ (u32)isi->active->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_P_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_P_CH);
+ }
}
return IRQ_HANDLED;
}
@@ -230,7 +239,8 @@ static irqreturn_t isi_interrupt(int irq, void *dev_id)
isi_writel(isi, ISI_INTDIS, ISI_CTRL_DIS);
ret = IRQ_HANDLED;
} else {
- if (likely(pending & ISI_SR_CXFR_DONE))
+ if (likely(pending & ISI_SR_CXFR_DONE) ||
+ likely(pending & ISI_SR_PXFR_DONE))
ret = atmel_isi_handle_streaming(isi);
}
@@ -372,21 +382,35 @@ static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
ISI_SR_CXFR_DONE | ISI_SR_PXFR_DONE);
/* Check if already in a frame */
- if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
- dev_err(isi->soc_host.icd->parent, "Already in frame handling.\n");
- return;
- }
+ if (!isi->enable_preview_path) {
+ if (isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) {
+ dev_err(isi->soc_host.icd->parent, "Already in frame handling.\n");
+ return;
+ }
- isi_writel(isi, ISI_DMA_C_DSCR, (u32)buffer->p_dma_desc->fbd_phys);
- isi_writel(isi, ISI_DMA_C_CTRL, ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
- isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ isi_writel(isi, ISI_DMA_C_DSCR,
+ (u32)buffer->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_C_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_C_CH);
+ } else {
+ isi_writel(isi, ISI_DMA_P_DSCR,
+ (u32)buffer->p_dma_desc->fbd_phys);
+ isi_writel(isi, ISI_DMA_P_CTRL,
+ ISI_DMA_CTRL_FETCH | ISI_DMA_CTRL_DONE);
+ isi_writel(isi, ISI_DMA_CHER, ISI_DMA_CHSR_P_CH);
+ }
cfg1 &= ~ISI_CFG1_FRATE_DIV_MASK;
/* Enable linked list */
cfg1 |= isi->pdata.frate | ISI_CFG1_DISCR;
- /* Enable codec path and ISI */
- ctrl = ISI_CTRL_CDC | ISI_CTRL_EN;
+ /* Enable ISI */
+ ctrl = ISI_CTRL_EN;
+
+ if (!isi->enable_preview_path)
+ ctrl |= ISI_CTRL_CDC;
+
isi_writel(isi, ISI_CTRL, ctrl);
isi_writel(isi, ISI_CFG1, cfg1);
}
@@ -462,15 +486,17 @@ static void stop_streaming(struct vb2_queue *vq)
}
spin_unlock_irq(&isi->lock);
- timeout = jiffies + FRAME_INTERVAL_MILLI_SEC * HZ;
- /* Wait until the end of the current frame. */
- while ((isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) &&
- time_before(jiffies, timeout))
- msleep(1);
+ if (!isi->enable_preview_path) {
+ timeout = jiffies + FRAME_INTERVAL_MILLI_SEC * HZ;
+ /* Wait until the end of the current frame. */
+ while ((isi_readl(isi, ISI_STATUS) & ISI_CTRL_CDC) &&
+ time_before(jiffies, timeout))
+ msleep(1);
- if (time_after(jiffies, timeout))
- dev_err(icd->parent,
- "Timeout waiting for finishing codec request\n");
+ if (time_after(jiffies, timeout))
+ dev_err(icd->parent,
+ "Timeout waiting for finishing codec request\n");
+ }
/* Disable interrupts */
isi_writel(isi, ISI_INTDIS,
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] media: atmel-isi: add code to setup correct resolution for preview path
2015-09-22 5:14 [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format Josh Wu
2015-09-22 5:14 ` [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs Josh Wu
2015-09-22 5:14 ` [PATCH 2/5] media: atmel-isi: prepare for the support of preview path Josh Wu
@ 2015-09-22 5:14 ` Josh Wu
2015-09-22 5:14 ` [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using " Josh Wu
2015-09-22 5:14 ` [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats Josh Wu
4 siblings, 0 replies; 18+ messages in thread
From: Josh Wu @ 2015-09-22 5:14 UTC (permalink / raw)
To: Linux Media Mailing List, linux-arm-kernel, Guennadi Liakhovetski
Cc: Laurent Pinchart, Josh Wu
Not like codec path, preview path can do downsampling, so we should setup
a extra preview width, height for it.
This patch add preview resolution setup without down sampling. So currently
preview path will output same size as sensor output size.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 12 +++++++++++-
drivers/media/platform/soc_camera/atmel-isi.h | 10 ++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index e6f4ade..bbf6449 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -135,7 +135,7 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
static void configure_geometry(struct atmel_isi *isi, u32 width,
u32 height, const struct soc_camera_format_xlate *xlate)
{
- u32 cfg2;
+ u32 cfg2, psize;
/* According to sensor's output format to set cfg2 */
switch (xlate->code) {
@@ -163,6 +163,16 @@ static void configure_geometry(struct atmel_isi *isi, u32 width,
cfg2 |= ((height - 1) << ISI_CFG2_IM_VSIZE_OFFSET)
& ISI_CFG2_IM_VSIZE_MASK;
isi_writel(isi, ISI_CFG2, cfg2);
+
+ /* No down sampling, preview size equal to sensor output size */
+ psize = ((width - 1) << ISI_PSIZE_PREV_HSIZE_OFFSET) &
+ ISI_PSIZE_PREV_HSIZE_MASK;
+ psize |= ((height - 1) << ISI_PSIZE_PREV_VSIZE_OFFSET) &
+ ISI_PSIZE_PREV_VSIZE_MASK;
+ isi_writel(isi, ISI_PSIZE, psize);
+ isi_writel(isi, ISI_PDECF, ISI_PDECF_NO_SAMPLING);
+
+ return;
}
static bool is_supported(struct soc_camera_device *icd,
diff --git a/drivers/media/platform/soc_camera/atmel-isi.h b/drivers/media/platform/soc_camera/atmel-isi.h
index 5acc771..0acb32a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.h
+++ b/drivers/media/platform/soc_camera/atmel-isi.h
@@ -79,6 +79,16 @@
#define ISI_CFG2_IM_VSIZE_MASK (0x7FF << ISI_CFG2_IM_VSIZE_OFFSET)
#define ISI_CFG2_IM_HSIZE_MASK (0x7FF << ISI_CFG2_IM_HSIZE_OFFSET)
+/* Bitfields in PSIZE */
+#define ISI_PSIZE_PREV_VSIZE_OFFSET 0
+#define ISI_PSIZE_PREV_HSIZE_OFFSET 16
+#define ISI_PSIZE_PREV_VSIZE_MASK (0x3FF << ISI_PSIZE_PREV_VSIZE_OFFSET)
+#define ISI_PSIZE_PREV_HSIZE_MASK (0x3FF << ISI_PSIZE_PREV_HSIZE_OFFSET)
+
+/* Bitfields in PDECF */
+#define ISI_PDECF_DEC_FACTOR_MASK (0xFF << 0)
+#define ISI_PDECF_NO_SAMPLING (16)
+
/* Bitfields in CTRL */
/* Also using in SR(ISI_V2) */
#define ISI_CTRL_EN (1 << 0)
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using preview path
2015-09-22 5:14 [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format Josh Wu
` (2 preceding siblings ...)
2015-09-22 5:14 ` [PATCH 3/5] media: atmel-isi: add code to setup correct resolution for " Josh Wu
@ 2015-09-22 5:14 ` Josh Wu
2015-10-04 16:50 ` Guennadi Liakhovetski
2015-09-22 5:14 ` [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats Josh Wu
4 siblings, 1 reply; 18+ messages in thread
From: Josh Wu @ 2015-09-22 5:14 UTC (permalink / raw)
To: Linux Media Mailing List, linux-arm-kernel, Guennadi Liakhovetski
Cc: Laurent Pinchart, Josh Wu
The preview path only can convert UYVY format to RGB data.
To make preview path work correctly, we need to set up YCC_SWAP
according to sensor output and convert them to UYVY.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index bbf6449..e87d354 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -127,6 +127,22 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
break;
}
+ } else if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_RGB565) {
+ /* Preview path is enabled, it will convert UYVY to RGB format.
+ * But if sensor output format is not UYVY, we need to set
+ * YCC_SWAP_MODE to convert it as UYVY.
+ */
+ switch (xlate->code) {
+ case MEDIA_BUS_FMT_VYUY8_2X8:
+ cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
+ break;
+ case MEDIA_BUS_FMT_YUYV8_2X8:
+ cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
+ break;
+ case MEDIA_BUS_FMT_YVYU8_2X8:
+ cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
+ break;
+ }
}
return cfg2_yuv_swap;
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
2015-09-22 5:14 [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format Josh Wu
` (3 preceding siblings ...)
2015-09-22 5:14 ` [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using " Josh Wu
@ 2015-09-22 5:14 ` Josh Wu
2015-10-04 17:02 ` Guennadi Liakhovetski
4 siblings, 1 reply; 18+ messages in thread
From: Josh Wu @ 2015-09-22 5:14 UTC (permalink / raw)
To: Linux Media Mailing List, linux-arm-kernel, Guennadi Liakhovetski
Cc: Laurent Pinchart, Josh Wu
This patch enable Atmel ISI preview path to convert the YUV to RGB format.
Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
drivers/media/platform/soc_camera/atmel-isi.c | 38 ++++++++++++++++++++-------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index e87d354..e33a16a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -201,13 +201,20 @@ static bool is_supported(struct soc_camera_device *icd,
case V4L2_PIX_FMT_UYVY:
case V4L2_PIX_FMT_YVYU:
case V4L2_PIX_FMT_VYUY:
+ /* RGB */
+ case V4L2_PIX_FMT_RGB565:
return true;
- /* RGB, TODO */
default:
return false;
}
}
+static bool is_output_rgb(const struct soc_mbus_pixelfmt *host_fmt)
+{
+ return host_fmt->fourcc == V4L2_PIX_FMT_RGB565 ||
+ host_fmt->fourcc == V4L2_PIX_FMT_RGB32;
+}
+
static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
{
if (isi->active) {
@@ -467,6 +474,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
struct atmel_isi *isi = ici->priv;
int ret;
+ isi->enable_preview_path = is_output_rgb(icd->current_fmt->host_fmt);
+
pm_runtime_get_sync(ici->v4l2_dev.dev);
/* Reset ISI */
@@ -688,6 +697,14 @@ static const struct soc_mbus_pixelfmt isi_camera_formats[] = {
.order = SOC_MBUS_ORDER_LE,
.layout = SOC_MBUS_LAYOUT_PACKED,
},
+ {
+ .fourcc = V4L2_PIX_FMT_RGB565,
+ .name = "RGB565",
+ .bits_per_sample = 8,
+ .packing = SOC_MBUS_PACKING_2X8_PADHI,
+ .order = SOC_MBUS_ORDER_LE,
+ .layout = SOC_MBUS_LAYOUT_PACKED,
+ },
};
/* This will be corrected as we get more formats */
@@ -744,7 +761,7 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
struct soc_camera_format_xlate *xlate)
{
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- int formats = 0, ret;
+ int formats = 0, ret, i, n;
/* sensor format */
struct v4l2_subdev_mbus_code_enum code = {
.which = V4L2_SUBDEV_FORMAT_ACTIVE,
@@ -778,13 +795,16 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
case MEDIA_BUS_FMT_VYUY8_2X8:
case MEDIA_BUS_FMT_YUYV8_2X8:
case MEDIA_BUS_FMT_YVYU8_2X8:
- formats++;
- if (xlate) {
- xlate->host_fmt = &isi_camera_formats[0];
- xlate->code = code.code;
- xlate++;
- dev_dbg(icd->parent, "Providing format %s using code %d\n",
- isi_camera_formats[0].name, code.code);
+ n = ARRAY_SIZE(isi_camera_formats);
+ formats += n;
+ for (i = 0; i < n; i++) {
+ if (xlate) {
+ xlate->host_fmt = &isi_camera_formats[i];
+ xlate->code = code.code;
+ dev_dbg(icd->parent, "Providing format %s using code %d\n",
+ isi_camera_formats[0].name, code.code);
+ xlate++;
+ }
}
break;
default:
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-09-22 5:14 ` [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs Josh Wu
@ 2015-10-04 16:43 ` Guennadi Liakhovetski
2015-10-04 17:04 ` Guennadi Liakhovetski
2015-10-14 6:43 ` Josh Wu
0 siblings, 2 replies; 18+ messages in thread
From: Guennadi Liakhovetski @ 2015-10-04 16:43 UTC (permalink / raw)
To: Josh Wu; +Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Hi Josh,
On Tue, 22 Sep 2015, Josh Wu wrote:
> we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
> sensor output and Atmel ISI output format.
>
> Current there are two cases Atmel ISI supported:
> 1. Atmel ISI outputs YUYV format.
> This case we need to setup YCC_SWAP according to sensor output
> format.
> 2. Atmel ISI output a pass-through formats, which means no swap.
> Just setup YCC_SWAP as default with no swap.
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>
> drivers/media/platform/soc_camera/atmel-isi.c | 43 ++++++++++++++++++++-------
> 1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index 45e304a..df64294 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
> return readl(isi->regs + reg);
> }
>
> +static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
> + const struct soc_camera_format_xlate *xlate)
> +{
This function will be called only for the four media codes from the
switch-case statement below, namely for
MEDIA_BUS_FMT_VYUY8_2X8
MEDIA_BUS_FMT_UYVY8_2X8
MEDIA_BUS_FMT_YVYU8_2X8
MEDIA_BUS_FMT_YUYV8_2X8
> + /* By default, no swap for the codec path of Atmel ISI. So codec
> + * output is same as sensor's output.
> + * For instance, if sensor's output is YUYV, then codec outputs YUYV.
> + * And if sensor's output is UYVY, then codec outputs UYVY.
> + */
> + u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
Then this ISI_CFG2_YCC_SWAP_DEFAULT will only hold for
MEDIA_BUS_FMT_YUYV8_2X8? Why don't you just add one more case below? Don't
think this initialisation is really justified.
> +
> + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
> + /* all convert to YUYV */
> + switch (xlate->code) {
> + case MEDIA_BUS_FMT_VYUY8_2X8:
> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
> + break;
> + case MEDIA_BUS_FMT_UYVY8_2X8:
> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
> + break;
> + case MEDIA_BUS_FMT_YVYU8_2X8:
> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
> + break;
> + }
> + }
> +
> + return cfg2_yuv_swap;
> +}
> +
> static void configure_geometry(struct atmel_isi *isi, u32 width,
> - u32 height, u32 code)
> + u32 height, const struct soc_camera_format_xlate *xlate)
> {
> u32 cfg2;
>
> /* According to sensor's output format to set cfg2 */
> - switch (code) {
> + switch (xlate->code) {
> default:
> /* Grey */
> case MEDIA_BUS_FMT_Y8_1X8:
> @@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi *isi, u32 width,
> break;
> /* YUV */
> case MEDIA_BUS_FMT_VYUY8_2X8:
> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
> - break;
> case MEDIA_BUS_FMT_UYVY8_2X8:
> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
> - break;
> case MEDIA_BUS_FMT_YVYU8_2X8:
> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
> - break;
> case MEDIA_BUS_FMT_YUYV8_2X8:
> - cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
> + cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
> + setup_cfg2_yuv_swap(isi, xlate);
> break;
> /* RGB, TODO */
> }
I would move this switch-case completely inside setup_cfg2_yuv_swap().
Just do
cfg2 = setup_cfg2_yuv_swap(isi, xlate);
and handle the
case MEDIA_BUS_FMT_Y8_1X8:
in the function too. These two switch-case statements really look
redundant.
> @@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> isi_writel(isi, ISI_INTDIS, (u32)~0UL);
>
> configure_geometry(isi, icd->user_width, icd->user_height,
> - icd->current_fmt->code);
> + icd->current_fmt);
>
> spin_lock_irq(&isi->lock);
> /* Clear any pending interrupt */
> --
> 1.9.1
>
Thanks
Guennadi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using preview path
2015-09-22 5:14 ` [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using " Josh Wu
@ 2015-10-04 16:50 ` Guennadi Liakhovetski
2015-10-14 6:44 ` Josh Wu
0 siblings, 1 reply; 18+ messages in thread
From: Guennadi Liakhovetski @ 2015-10-04 16:50 UTC (permalink / raw)
To: Josh Wu; +Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
On Tue, 22 Sep 2015, Josh Wu wrote:
> The preview path only can convert UYVY format to RGB data.
>
> To make preview path work correctly, we need to set up YCC_SWAP
> according to sensor output and convert them to UYVY.
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>
> drivers/media/platform/soc_camera/atmel-isi.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index bbf6449..e87d354 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -127,6 +127,22 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
> cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
> break;
> }
> + } else if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_RGB565) {
> + /* Preview path is enabled, it will convert UYVY to RGB format.
> + * But if sensor output format is not UYVY, we need to set
> + * YCC_SWAP_MODE to convert it as UYVY.
> + */
Please, fix multiline comment style:
/*
* ...
* ...
*/
> + switch (xlate->code) {
> + case MEDIA_BUS_FMT_VYUY8_2X8:
> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
> + break;
> + case MEDIA_BUS_FMT_YUYV8_2X8:
> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
> + break;
> + case MEDIA_BUS_FMT_YVYU8_2X8:
> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
> + break;
> + }
> }
>
> return cfg2_yuv_swap;
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
2015-09-22 5:14 ` [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats Josh Wu
@ 2015-10-04 17:02 ` Guennadi Liakhovetski
2015-10-14 6:57 ` Josh Wu
0 siblings, 1 reply; 18+ messages in thread
From: Guennadi Liakhovetski @ 2015-10-04 17:02 UTC (permalink / raw)
To: Josh Wu; +Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
On Tue, 22 Sep 2015, Josh Wu wrote:
> This patch enable Atmel ISI preview path to convert the YUV to RGB format.
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>
> drivers/media/platform/soc_camera/atmel-isi.c | 38 ++++++++++++++++++++-------
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index e87d354..e33a16a 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -201,13 +201,20 @@ static bool is_supported(struct soc_camera_device *icd,
> case V4L2_PIX_FMT_UYVY:
> case V4L2_PIX_FMT_YVYU:
> case V4L2_PIX_FMT_VYUY:
> + /* RGB */
> + case V4L2_PIX_FMT_RGB565:
> return true;
> - /* RGB, TODO */
> default:
> return false;
> }
> }
>
> +static bool is_output_rgb(const struct soc_mbus_pixelfmt *host_fmt)
> +{
> + return host_fmt->fourcc == V4L2_PIX_FMT_RGB565 ||
> + host_fmt->fourcc == V4L2_PIX_FMT_RGB32;
> +}
> +
Why not just pass fourcc to this function? Or maybe just embed it in
start_streaming - it won't clutter it a lot.
> static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
> {
> if (isi->active) {
> @@ -467,6 +474,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> struct atmel_isi *isi = ici->priv;
> int ret;
>
> + isi->enable_preview_path = is_output_rgb(icd->current_fmt->host_fmt);
> +
> pm_runtime_get_sync(ici->v4l2_dev.dev);
>
> /* Reset ISI */
> @@ -688,6 +697,14 @@ static const struct soc_mbus_pixelfmt isi_camera_formats[] = {
> .order = SOC_MBUS_ORDER_LE,
> .layout = SOC_MBUS_LAYOUT_PACKED,
> },
> + {
> + .fourcc = V4L2_PIX_FMT_RGB565,
> + .name = "RGB565",
> + .bits_per_sample = 8,
> + .packing = SOC_MBUS_PACKING_2X8_PADHI,
> + .order = SOC_MBUS_ORDER_LE,
> + .layout = SOC_MBUS_LAYOUT_PACKED,
> + },
> };
>
> /* This will be corrected as we get more formats */
> @@ -744,7 +761,7 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
> struct soc_camera_format_xlate *xlate)
> {
> struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> - int formats = 0, ret;
> + int formats = 0, ret, i, n;
> /* sensor format */
> struct v4l2_subdev_mbus_code_enum code = {
> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> @@ -778,13 +795,16 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
> case MEDIA_BUS_FMT_VYUY8_2X8:
> case MEDIA_BUS_FMT_YUYV8_2X8:
> case MEDIA_BUS_FMT_YVYU8_2X8:
> - formats++;
> - if (xlate) {
> - xlate->host_fmt = &isi_camera_formats[0];
> - xlate->code = code.code;
> - xlate++;
> - dev_dbg(icd->parent, "Providing format %s using code %d\n",
> - isi_camera_formats[0].name, code.code);
> + n = ARRAY_SIZE(isi_camera_formats);
> + formats += n;
> + for (i = 0; i < n; i++) {
> + if (xlate) {
I'd put if outside of the loop, or just do
+ for (i = 0; xlate && i < n; i++) {
> + xlate->host_fmt = &isi_camera_formats[i];
> + xlate->code = code.code;
> + dev_dbg(icd->parent, "Providing format %s using code %d\n",
> + isi_camera_formats[0].name, code.code);
> + xlate++;
> + }
> }
> break;
> default:
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-10-04 16:43 ` Guennadi Liakhovetski
@ 2015-10-04 17:04 ` Guennadi Liakhovetski
2015-10-14 6:46 ` Josh Wu
2015-10-14 6:43 ` Josh Wu
1 sibling, 1 reply; 18+ messages in thread
From: Guennadi Liakhovetski @ 2015-10-04 17:04 UTC (permalink / raw)
To: Josh Wu; +Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Even simpler:
On Sun, 4 Oct 2015, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> On Tue, 22 Sep 2015, Josh Wu wrote:
>
> > we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
> > sensor output and Atmel ISI output format.
> >
> > Current there are two cases Atmel ISI supported:
> > 1. Atmel ISI outputs YUYV format.
> > This case we need to setup YCC_SWAP according to sensor output
> > format.
> > 2. Atmel ISI output a pass-through formats, which means no swap.
> > Just setup YCC_SWAP as default with no swap.
> >
> > Signed-off-by: Josh Wu <josh.wu@atmel.com>
> > ---
> >
> > drivers/media/platform/soc_camera/atmel-isi.c | 43 ++++++++++++++++++++-------
> > 1 file changed, 33 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> > index 45e304a..df64294 100644
> > --- a/drivers/media/platform/soc_camera/atmel-isi.c
> > +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> > @@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
> > return readl(isi->regs + reg);
> > }
> >
> > +static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
> > + const struct soc_camera_format_xlate *xlate)
> > +{
>
> This function will be called only for the four media codes from the
> switch-case statement below, namely for
>
> MEDIA_BUS_FMT_VYUY8_2X8
> MEDIA_BUS_FMT_UYVY8_2X8
> MEDIA_BUS_FMT_YVYU8_2X8
> MEDIA_BUS_FMT_YUYV8_2X8
>
> > + /* By default, no swap for the codec path of Atmel ISI. So codec
> > + * output is same as sensor's output.
> > + * For instance, if sensor's output is YUYV, then codec outputs YUYV.
> > + * And if sensor's output is UYVY, then codec outputs UYVY.
> > + */
> > + u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
>
> Then this ISI_CFG2_YCC_SWAP_DEFAULT will only hold for
> MEDIA_BUS_FMT_YUYV8_2X8? Why don't you just add one more case below? Don't
> think this initialisation is really justified.
>
> > +
> > + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
> > + /* all convert to YUYV */
> > + switch (xlate->code) {
> > + case MEDIA_BUS_FMT_VYUY8_2X8:
> > + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
> > + break;
Why don't you just return the result value here directly? Then you don't
need the variable at all
> > + case MEDIA_BUS_FMT_UYVY8_2X8:
> > + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
> > + break;
> > + case MEDIA_BUS_FMT_YVYU8_2X8:
> > + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
> > + break;
> > + }
> > + }
> > +
> > + return cfg2_yuv_swap;
> > +}
> > +
> > static void configure_geometry(struct atmel_isi *isi, u32 width,
> > - u32 height, u32 code)
> > + u32 height, const struct soc_camera_format_xlate *xlate)
> > {
> > u32 cfg2;
> >
> > /* According to sensor's output format to set cfg2 */
> > - switch (code) {
> > + switch (xlate->code) {
> > default:
> > /* Grey */
> > case MEDIA_BUS_FMT_Y8_1X8:
> > @@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi *isi, u32 width,
> > break;
> > /* YUV */
> > case MEDIA_BUS_FMT_VYUY8_2X8:
> > - cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
> > - break;
> > case MEDIA_BUS_FMT_UYVY8_2X8:
> > - cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
> > - break;
> > case MEDIA_BUS_FMT_YVYU8_2X8:
> > - cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
> > - break;
> > case MEDIA_BUS_FMT_YUYV8_2X8:
> > - cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
> > + cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
> > + setup_cfg2_yuv_swap(isi, xlate);
> > break;
> > /* RGB, TODO */
> > }
>
> I would move this switch-case completely inside setup_cfg2_yuv_swap().
> Just do
>
> cfg2 = setup_cfg2_yuv_swap(isi, xlate);
>
> and handle the
>
> case MEDIA_BUS_FMT_Y8_1X8:
>
> in the function too. These two switch-case statements really look
> redundant.
>
> > @@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> > isi_writel(isi, ISI_INTDIS, (u32)~0UL);
> >
> > configure_geometry(isi, icd->user_width, icd->user_height,
> > - icd->current_fmt->code);
> > + icd->current_fmt);
> >
> > spin_lock_irq(&isi->lock);
> > /* Clear any pending interrupt */
> > --
> > 1.9.1
> >
>
> Thanks
> Guennadi
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-10-04 16:43 ` Guennadi Liakhovetski
2015-10-04 17:04 ` Guennadi Liakhovetski
@ 2015-10-14 6:43 ` Josh Wu
2015-10-19 1:48 ` Guennadi Liakhovetski
1 sibling, 1 reply; 18+ messages in thread
From: Josh Wu @ 2015-10-14 6:43 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Hi, Dear Guennadi
Thanks for the review.
On 10/5/2015 12:43 AM, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> On Tue, 22 Sep 2015, Josh Wu wrote:
>
>> we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
>> sensor output and Atmel ISI output format.
>>
>> Current there are two cases Atmel ISI supported:
>> 1. Atmel ISI outputs YUYV format.
>> This case we need to setup YCC_SWAP according to sensor output
>> format.
>> 2. Atmel ISI output a pass-through formats, which means no swap.
>> Just setup YCC_SWAP as default with no swap.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>
>> drivers/media/platform/soc_camera/atmel-isi.c | 43 ++++++++++++++++++++-------
>> 1 file changed, 33 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index 45e304a..df64294 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
>> return readl(isi->regs + reg);
>> }
>>
>> +static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
>> + const struct soc_camera_format_xlate *xlate)
>> +{
> This function will be called only for the four media codes from the
> switch-case statement below, namely for
>
> MEDIA_BUS_FMT_VYUY8_2X8
> MEDIA_BUS_FMT_UYVY8_2X8
> MEDIA_BUS_FMT_YVYU8_2X8
> MEDIA_BUS_FMT_YUYV8_2X8
>
>> + /* By default, no swap for the codec path of Atmel ISI. So codec
>> + * output is same as sensor's output.
>> + * For instance, if sensor's output is YUYV, then codec outputs YUYV.
>> + * And if sensor's output is UYVY, then codec outputs UYVY.
>> + */
>> + u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
> Then this ISI_CFG2_YCC_SWAP_DEFAULT will only hold for
> MEDIA_BUS_FMT_YUYV8_2X8? Why don't you just add one more case below? Don't
> think this initialisation is really justified.
This default initial value is for all host_fmt_fourcc case. Not just for
V4L2_PIX_FMT_YUYV this case.
>
>> +
>> + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
>> + /* all convert to YUYV */
>> + switch (xlate->code) {
>> + case MEDIA_BUS_FMT_VYUY8_2X8:
>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
>> + break;
>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
>> + break;
>> + case MEDIA_BUS_FMT_YVYU8_2X8:
>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
>> + break;
>> + }
>> + }
>> +
>> + return cfg2_yuv_swap;
>> +}
>> +
>> static void configure_geometry(struct atmel_isi *isi, u32 width,
>> - u32 height, u32 code)
>> + u32 height, const struct soc_camera_format_xlate *xlate)
>> {
>> u32 cfg2;
>>
>> /* According to sensor's output format to set cfg2 */
>> - switch (code) {
>> + switch (xlate->code) {
>> default:
>> /* Grey */
>> case MEDIA_BUS_FMT_Y8_1X8:
>> @@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi *isi, u32 width,
>> break;
>> /* YUV */
>> case MEDIA_BUS_FMT_VYUY8_2X8:
>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
>> - break;
>> case MEDIA_BUS_FMT_UYVY8_2X8:
>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
>> - break;
>> case MEDIA_BUS_FMT_YVYU8_2X8:
>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
>> - break;
>> case MEDIA_BUS_FMT_YUYV8_2X8:
>> - cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
>> + cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
>> + setup_cfg2_yuv_swap(isi, xlate);
>> break;
>> /* RGB, TODO */
>> }
> I would move this switch-case completely inside setup_cfg2_yuv_swap().
> Just do
>
> cfg2 = setup_cfg2_yuv_swap(isi, xlate);
>
> and handle the
>
> case MEDIA_BUS_FMT_Y8_1X8:
>
> in the function too. These two switch-case statements really look
> redundant.
Technically, you can do that. But for my point of view, the
setup_cfg2_yuv_swap() only need to setup the yuv swap register field.
And other cfg2 field need to be configured as well, especially in the
case sensor output a RGB data. That should be implement soon.
>
>> @@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>> isi_writel(isi, ISI_INTDIS, (u32)~0UL);
>>
>> configure_geometry(isi, icd->user_width, icd->user_height,
>> - icd->current_fmt->code);
>> + icd->current_fmt);
>>
>> spin_lock_irq(&isi->lock);
>> /* Clear any pending interrupt */
>> --
>> 1.9.1
>>
> Thanks
> Guennadi
Best Regards,
Josh Wu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using preview path
2015-10-04 16:50 ` Guennadi Liakhovetski
@ 2015-10-14 6:44 ` Josh Wu
0 siblings, 0 replies; 18+ messages in thread
From: Josh Wu @ 2015-10-14 6:44 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Hi, Dear Guennadi
On 10/5/2015 12:50 AM, Guennadi Liakhovetski wrote:
> On Tue, 22 Sep 2015, Josh Wu wrote:
>
>> The preview path only can convert UYVY format to RGB data.
>>
>> To make preview path work correctly, we need to set up YCC_SWAP
>> according to sensor output and convert them to UYVY.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>
>> drivers/media/platform/soc_camera/atmel-isi.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index bbf6449..e87d354 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -127,6 +127,22 @@ static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
>> cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
>> break;
>> }
>> + } else if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_RGB565) {
>> + /* Preview path is enabled, it will convert UYVY to RGB format.
>> + * But if sensor output format is not UYVY, we need to set
>> + * YCC_SWAP_MODE to convert it as UYVY.
>> + */
> Please, fix multiline comment style:
>
> /*
> * ...
> * ...
> */
oh, yes, Sure. I'll fix this. Thanks.
Best Regards,
Josh Wu
>> + switch (xlate->code) {
>> + case MEDIA_BUS_FMT_VYUY8_2X8:
>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
>> + break;
>> + case MEDIA_BUS_FMT_YUYV8_2X8:
>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
>> + break;
>> + case MEDIA_BUS_FMT_YVYU8_2X8:
>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
>> + break;
>> + }
>> }
>>
>> return cfg2_yuv_swap;
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-10-04 17:04 ` Guennadi Liakhovetski
@ 2015-10-14 6:46 ` Josh Wu
0 siblings, 0 replies; 18+ messages in thread
From: Josh Wu @ 2015-10-14 6:46 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Dear Gueannadi,
On 10/5/2015 1:04 AM, Guennadi Liakhovetski wrote:
> Even simpler:
>
> On Sun, 4 Oct 2015, Guennadi Liakhovetski wrote:
>
>> Hi Josh,
>>
>> On Tue, 22 Sep 2015, Josh Wu wrote:
>>
>>> we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
>>> sensor output and Atmel ISI output format.
>>>
>>> Current there are two cases Atmel ISI supported:
>>> 1. Atmel ISI outputs YUYV format.
>>> This case we need to setup YCC_SWAP according to sensor output
>>> format.
>>> 2. Atmel ISI output a pass-through formats, which means no swap.
>>> Just setup YCC_SWAP as default with no swap.
>>>
>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>> ---
>>>
>>> drivers/media/platform/soc_camera/atmel-isi.c | 43 ++++++++++++++++++++-------
>>> 1 file changed, 33 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>>> index 45e304a..df64294 100644
>>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>>> @@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
>>> return readl(isi->regs + reg);
>>> }
>>>
>>> +static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
>>> + const struct soc_camera_format_xlate *xlate)
>>> +{
>> This function will be called only for the four media codes from the
>> switch-case statement below, namely for
>>
>> MEDIA_BUS_FMT_VYUY8_2X8
>> MEDIA_BUS_FMT_UYVY8_2X8
>> MEDIA_BUS_FMT_YVYU8_2X8
>> MEDIA_BUS_FMT_YUYV8_2X8
>>
>>> + /* By default, no swap for the codec path of Atmel ISI. So codec
>>> + * output is same as sensor's output.
>>> + * For instance, if sensor's output is YUYV, then codec outputs YUYV.
>>> + * And if sensor's output is UYVY, then codec outputs UYVY.
>>> + */
>>> + u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
>> Then this ISI_CFG2_YCC_SWAP_DEFAULT will only hold for
>> MEDIA_BUS_FMT_YUYV8_2X8? Why don't you just add one more case below? Don't
>> think this initialisation is really justified.
>>
>>> +
>>> + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
>>> + /* all convert to YUYV */
>>> + switch (xlate->code) {
>>> + case MEDIA_BUS_FMT_VYUY8_2X8:
>>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
>>> + break;
> Why don't you just return the result value here directly? Then you don't
> need the variable at all
yes, This sounds good. I'll take rid of the cfg2_yuv_swap variable. Thanks.
Best Regards,
Josh Wu
>
>>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
>>> + break;
>>> + case MEDIA_BUS_FMT_YVYU8_2X8:
>>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + return cfg2_yuv_swap;
>>> +}
>>> +
>>> static void configure_geometry(struct atmel_isi *isi, u32 width,
>>> - u32 height, u32 code)
>>> + u32 height, const struct soc_camera_format_xlate *xlate)
>>> {
>>> u32 cfg2;
>>>
>>> /* According to sensor's output format to set cfg2 */
>>> - switch (code) {
>>> + switch (xlate->code) {
>>> default:
>>> /* Grey */
>>> case MEDIA_BUS_FMT_Y8_1X8:
>>> @@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi *isi, u32 width,
>>> break;
>>> /* YUV */
>>> case MEDIA_BUS_FMT_VYUY8_2X8:
>>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
>>> - break;
>>> case MEDIA_BUS_FMT_UYVY8_2X8:
>>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
>>> - break;
>>> case MEDIA_BUS_FMT_YVYU8_2X8:
>>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
>>> - break;
>>> case MEDIA_BUS_FMT_YUYV8_2X8:
>>> - cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
>>> + cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
>>> + setup_cfg2_yuv_swap(isi, xlate);
>>> break;
>>> /* RGB, TODO */
>>> }
>> I would move this switch-case completely inside setup_cfg2_yuv_swap().
>> Just do
>>
>> cfg2 = setup_cfg2_yuv_swap(isi, xlate);
>>
>> and handle the
>>
>> case MEDIA_BUS_FMT_Y8_1X8:
>>
>> in the function too. These two switch-case statements really look
>> redundant.
>>
>>> @@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>>> isi_writel(isi, ISI_INTDIS, (u32)~0UL);
>>>
>>> configure_geometry(isi, icd->user_width, icd->user_height,
>>> - icd->current_fmt->code);
>>> + icd->current_fmt);
>>>
>>> spin_lock_irq(&isi->lock);
>>> /* Clear any pending interrupt */
>>> --
>>> 1.9.1
>>>
>> Thanks
>> Guennadi
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
2015-10-04 17:02 ` Guennadi Liakhovetski
@ 2015-10-14 6:57 ` Josh Wu
2015-10-19 2:03 ` Guennadi Liakhovetski
0 siblings, 1 reply; 18+ messages in thread
From: Josh Wu @ 2015-10-14 6:57 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Dear Guennadi,
Thanks for the review.
On 10/5/2015 1:02 AM, Guennadi Liakhovetski wrote:
> On Tue, 22 Sep 2015, Josh Wu wrote:
>
>> This patch enable Atmel ISI preview path to convert the YUV to RGB format.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>
>> drivers/media/platform/soc_camera/atmel-isi.c | 38 ++++++++++++++++++++-------
>> 1 file changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index e87d354..e33a16a 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -201,13 +201,20 @@ static bool is_supported(struct soc_camera_device *icd,
>> case V4L2_PIX_FMT_UYVY:
>> case V4L2_PIX_FMT_YVYU:
>> case V4L2_PIX_FMT_VYUY:
>> + /* RGB */
>> + case V4L2_PIX_FMT_RGB565:
>> return true;
>> - /* RGB, TODO */
>> default:
>> return false;
>> }
>> }
>>
>> +static bool is_output_rgb(const struct soc_mbus_pixelfmt *host_fmt)
>> +{
>> + return host_fmt->fourcc == V4L2_PIX_FMT_RGB565 ||
>> + host_fmt->fourcc == V4L2_PIX_FMT_RGB32;
>> +}
>> +
> Why not just pass fourcc to this function? Or maybe just embed it in
> start_streaming - it won't clutter it a lot.
I think pass fourcc to the function is good.
Since configure_geometry() is hardware related, and the
enable_preview_path is also hardware related, so I prefer initialize
enable_preview_path in configure_geometry().
>
>> static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
>> {
>> if (isi->active) {
>> @@ -467,6 +474,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>> struct atmel_isi *isi = ici->priv;
>> int ret;
>>
>> + isi->enable_preview_path = is_output_rgb(icd->current_fmt->host_fmt);
>> +
>> pm_runtime_get_sync(ici->v4l2_dev.dev);
>>
>> /* Reset ISI */
>> @@ -688,6 +697,14 @@ static const struct soc_mbus_pixelfmt isi_camera_formats[] = {
>> .order = SOC_MBUS_ORDER_LE,
>> .layout = SOC_MBUS_LAYOUT_PACKED,
>> },
>> + {
>> + .fourcc = V4L2_PIX_FMT_RGB565,
>> + .name = "RGB565",
>> + .bits_per_sample = 8,
>> + .packing = SOC_MBUS_PACKING_2X8_PADHI,
>> + .order = SOC_MBUS_ORDER_LE,
>> + .layout = SOC_MBUS_LAYOUT_PACKED,
>> + },
>> };
>>
>> /* This will be corrected as we get more formats */
>> @@ -744,7 +761,7 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
>> struct soc_camera_format_xlate *xlate)
>> {
>> struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>> - int formats = 0, ret;
>> + int formats = 0, ret, i, n;
>> /* sensor format */
>> struct v4l2_subdev_mbus_code_enum code = {
>> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
>> @@ -778,13 +795,16 @@ static int isi_camera_get_formats(struct soc_camera_device *icd,
>> case MEDIA_BUS_FMT_VYUY8_2X8:
>> case MEDIA_BUS_FMT_YUYV8_2X8:
>> case MEDIA_BUS_FMT_YVYU8_2X8:
>> - formats++;
>> - if (xlate) {
>> - xlate->host_fmt = &isi_camera_formats[0];
>> - xlate->code = code.code;
>> - xlate++;
>> - dev_dbg(icd->parent, "Providing format %s using code %d\n",
>> - isi_camera_formats[0].name, code.code);
>> + n = ARRAY_SIZE(isi_camera_formats);
>> + formats += n;
>> + for (i = 0; i < n; i++) {
>> + if (xlate) {
> I'd put if outside of the loop, or just do
>
> + for (i = 0; xlate && i < n; i++) {
yes, that simpler one. I'll take it. Thanks.
Best Regards,
Josh Wu
>
>
>> + xlate->host_fmt = &isi_camera_formats[i];
>> + xlate->code = code.code;
>> + dev_dbg(icd->parent, "Providing format %s using code %d\n",
>> + isi_camera_formats[0].name, code.code);
>> + xlate++;
>> + }
>> }
>> break;
>> default:
>> --
>> 1.9.1
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-10-14 6:43 ` Josh Wu
@ 2015-10-19 1:48 ` Guennadi Liakhovetski
2015-10-19 2:45 ` Josh Wu
0 siblings, 1 reply; 18+ messages in thread
From: Guennadi Liakhovetski @ 2015-10-19 1:48 UTC (permalink / raw)
To: Josh Wu; +Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Hi Josh,
On Wed, 14 Oct 2015, Josh Wu wrote:
> Hi, Dear Guennadi
>
> Thanks for the review.
>
> On 10/5/2015 12:43 AM, Guennadi Liakhovetski wrote:
> > Hi Josh,
> >
> > On Tue, 22 Sep 2015, Josh Wu wrote:
> >
> > > we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
> > > sensor output and Atmel ISI output format.
> > >
> > > Current there are two cases Atmel ISI supported:
> > > 1. Atmel ISI outputs YUYV format.
> > > This case we need to setup YCC_SWAP according to sensor output
> > > format.
> > > 2. Atmel ISI output a pass-through formats, which means no swap.
> > > Just setup YCC_SWAP as default with no swap.
> > >
> > > Signed-off-by: Josh Wu <josh.wu@atmel.com>
> > > ---
> > >
> > > drivers/media/platform/soc_camera/atmel-isi.c | 43
> > > ++++++++++++++++++++-------
> > > 1 file changed, 33 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
> > > b/drivers/media/platform/soc_camera/atmel-isi.c
> > > index 45e304a..df64294 100644
> > > --- a/drivers/media/platform/soc_camera/atmel-isi.c
> > > +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> > > @@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
> > > return readl(isi->regs + reg);
> > > }
> > > +static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
> > > + const struct soc_camera_format_xlate *xlate)
> > > +{
> > This function will be called only for the four media codes from the
> > switch-case statement below, namely for
> >
> > MEDIA_BUS_FMT_VYUY8_2X8
> > MEDIA_BUS_FMT_UYVY8_2X8
> > MEDIA_BUS_FMT_YVYU8_2X8
> > MEDIA_BUS_FMT_YUYV8_2X8
> >
> > > + /* By default, no swap for the codec path of Atmel ISI. So codec
> > > + * output is same as sensor's output.
> > > + * For instance, if sensor's output is YUYV, then codec outputs YUYV.
> > > + * And if sensor's output is UYVY, then codec outputs UYVY.
> > > + */
> > > + u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
> > Then this ISI_CFG2_YCC_SWAP_DEFAULT will only hold for
> > MEDIA_BUS_FMT_YUYV8_2X8? Why don't you just add one more case below? Don't
> > think this initialisation is really justified.
> This default initial value is for all host_fmt_fourcc case. Not just for
> V4L2_PIX_FMT_YUYV this case.
Right, yes, missed this. How about this then:
if (xlate->host_fmt->fourcc != V4L2_PIX_FMT_YUYV)
return ISI_CFG2_YCC_SWAP_DEFAULT;
switch (xlate->code) {
case MEDIA_BUS_FMT_VYUY8_2X8:
return ISI_CFG2_YCC_SWAP_MODE_3;
case MEDIA_BUS_FMT_UYVY8_2X8:
return ISI_CFG2_YCC_SWAP_MODE_2;
case MEDIA_BUS_FMT_YVYU8_2X8:
return ISI_CFG2_YCC_SWAP_MODE_1;
}
return ISI_CFG2_YCC_SWAP_DEFAULT;
Or even exactly as you have it in your patch only remove the cfg2_yuv_swap
variable and do "return" directly after each "case MEDIA_..." and one more
with SWAP_DEFAULT in the end.
> > > +
> > > + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
> > > + /* all convert to YUYV */
> > > + switch (xlate->code) {
> > > + case MEDIA_BUS_FMT_VYUY8_2X8:
> > > + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
> > > + break;
> > > + case MEDIA_BUS_FMT_UYVY8_2X8:
> > > + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
> > > + break;
> > > + case MEDIA_BUS_FMT_YVYU8_2X8:
> > > + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return cfg2_yuv_swap;
> > > +}
> > > +
> > > static void configure_geometry(struct atmel_isi *isi, u32 width,
> > > - u32 height, u32 code)
> > > + u32 height, const struct soc_camera_format_xlate *xlate)
> > > {
> > > u32 cfg2;
> > > /* According to sensor's output format to set cfg2 */
> > > - switch (code) {
> > > + switch (xlate->code) {
> > > default:
> > > /* Grey */
> > > case MEDIA_BUS_FMT_Y8_1X8:
> > > @@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi
> > > *isi, u32 width,
> > > break;
> > > /* YUV */
> > > case MEDIA_BUS_FMT_VYUY8_2X8:
> > > - cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
> > > - break;
> > > case MEDIA_BUS_FMT_UYVY8_2X8:
> > > - cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
> > > - break;
> > > case MEDIA_BUS_FMT_YVYU8_2X8:
> > > - cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
> > > - break;
> > > case MEDIA_BUS_FMT_YUYV8_2X8:
> > > - cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
> > > + cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
> > > + setup_cfg2_yuv_swap(isi, xlate);
> > > break;
> > > /* RGB, TODO */
> > > }
> > I would move this switch-case completely inside setup_cfg2_yuv_swap().
> > Just do
> >
> > cfg2 = setup_cfg2_yuv_swap(isi, xlate);
> >
> > and handle the
> >
> > case MEDIA_BUS_FMT_Y8_1X8:
> >
> > in the function too. These two switch-case statements really look
> > redundant.
> Technically, you can do that. But for my point of view, the
> setup_cfg2_yuv_swap() only need to setup the yuv swap register field.
>
> And other cfg2 field need to be configured as well, especially in the case
> sensor output a RGB data. That should be implement soon.
Understand. I don't like redundant code and this certainly looks redundant
to me. I'd be glad if you could remove this redundancy, but if you feel
strongly about it, feel free to keep as is.
Thanks
Guennadi
> > > @@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq,
> > > unsigned int count)
> > > isi_writel(isi, ISI_INTDIS, (u32)~0UL);
> > > configure_geometry(isi, icd->user_width, icd->user_height,
> > > - icd->current_fmt->code);
> > > + icd->current_fmt);
> > > spin_lock_irq(&isi->lock);
> > > /* Clear any pending interrupt */
> > > --
> > > 1.9.1
> > >
> > Thanks
> > Guennadi
>
> Best Regards,
> Josh Wu
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
2015-10-14 6:57 ` Josh Wu
@ 2015-10-19 2:03 ` Guennadi Liakhovetski
2015-10-19 2:50 ` Josh Wu
0 siblings, 1 reply; 18+ messages in thread
From: Guennadi Liakhovetski @ 2015-10-19 2:03 UTC (permalink / raw)
To: Josh Wu; +Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Hi Josh,
On Wed, 14 Oct 2015, Josh Wu wrote:
> Dear Guennadi,
>
> Thanks for the review.
>
> On 10/5/2015 1:02 AM, Guennadi Liakhovetski wrote:
> > On Tue, 22 Sep 2015, Josh Wu wrote:
> >
> > > This patch enable Atmel ISI preview path to convert the YUV to RGB format.
> > >
> > > Signed-off-by: Josh Wu <josh.wu@atmel.com>
> > > ---
> > >
> > > drivers/media/platform/soc_camera/atmel-isi.c | 38
> > > ++++++++++++++++++++-------
> > > 1 file changed, 29 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
> > > b/drivers/media/platform/soc_camera/atmel-isi.c
> > > index e87d354..e33a16a 100644
> > > --- a/drivers/media/platform/soc_camera/atmel-isi.c
> > > +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> > > @@ -201,13 +201,20 @@ static bool is_supported(struct soc_camera_device
> > > *icd,
> > > case V4L2_PIX_FMT_UYVY:
> > > case V4L2_PIX_FMT_YVYU:
> > > case V4L2_PIX_FMT_VYUY:
> > > + /* RGB */
> > > + case V4L2_PIX_FMT_RGB565:
> > > return true;
> > > - /* RGB, TODO */
> > > default:
> > > return false;
> > > }
> > > }
> > > +static bool is_output_rgb(const struct soc_mbus_pixelfmt *host_fmt)
> > > +{
> > > + return host_fmt->fourcc == V4L2_PIX_FMT_RGB565 ||
> > > + host_fmt->fourcc == V4L2_PIX_FMT_RGB32;
> > > +}
> > > +
> > Why not just pass fourcc to this function? Or maybe just embed it in
> > start_streaming - it won't clutter it a lot.
>
> I think pass fourcc to the function is good.
> Since configure_geometry() is hardware related, and the enable_preview_path is
> also hardware related, so I prefer initialize enable_preview_path in
> configure_geometry().
But you don't, you do it in start_streaming() below. But actually my
comment was not about _where_ to do it, but whether this calculation is
worth a separate function. I would just perform this calculation directly
where you're calling it:
static ... start_streaming(...)
{
...
u32 fourcc = icd->current_fmt->host_fmt->fourcc;
isi->enable_preview_path = fourcc == V4L2_PIX_FMT_RGB565 ||
fourcc == V4L2_PIX_FMT_RGB32;
Thanks
Guennadi
> > > static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
> > > {
> > > if (isi->active) {
> > > @@ -467,6 +474,8 @@ static int start_streaming(struct vb2_queue *vq,
> > > unsigned int count)
> > > struct atmel_isi *isi = ici->priv;
> > > int ret;
> > > + isi->enable_preview_path = is_output_rgb(icd->current_fmt->host_fmt);
> > > +
> > > pm_runtime_get_sync(ici->v4l2_dev.dev);
> > > /* Reset ISI */
> > > @@ -688,6 +697,14 @@ static const struct soc_mbus_pixelfmt
> > > isi_camera_formats[] = {
> > > .order = SOC_MBUS_ORDER_LE,
> > > .layout = SOC_MBUS_LAYOUT_PACKED,
> > > },
> > > + {
> > > + .fourcc = V4L2_PIX_FMT_RGB565,
> > > + .name = "RGB565",
> > > + .bits_per_sample = 8,
> > > + .packing = SOC_MBUS_PACKING_2X8_PADHI,
> > > + .order = SOC_MBUS_ORDER_LE,
> > > + .layout = SOC_MBUS_LAYOUT_PACKED,
> > > + },
> > > };
> > > /* This will be corrected as we get more formats */
> > > @@ -744,7 +761,7 @@ static int isi_camera_get_formats(struct
> > > soc_camera_device *icd,
> > > struct soc_camera_format_xlate *xlate)
> > > {
> > > struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> > > - int formats = 0, ret;
> > > + int formats = 0, ret, i, n;
> > > /* sensor format */
> > > struct v4l2_subdev_mbus_code_enum code = {
> > > .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > > @@ -778,13 +795,16 @@ static int isi_camera_get_formats(struct
> > > soc_camera_device *icd,
> > > case MEDIA_BUS_FMT_VYUY8_2X8:
> > > case MEDIA_BUS_FMT_YUYV8_2X8:
> > > case MEDIA_BUS_FMT_YVYU8_2X8:
> > > - formats++;
> > > - if (xlate) {
> > > - xlate->host_fmt = &isi_camera_formats[0];
> > > - xlate->code = code.code;
> > > - xlate++;
> > > - dev_dbg(icd->parent, "Providing format %s using code
> > > %d\n",
> > > - isi_camera_formats[0].name, code.code);
> > > + n = ARRAY_SIZE(isi_camera_formats);
> > > + formats += n;
> > > + for (i = 0; i < n; i++) {
> > > + if (xlate) {
> > I'd put if outside of the loop, or just do
> >
> > + for (i = 0; xlate && i < n; i++) {
>
> yes, that simpler one. I'll take it. Thanks.
>
> Best Regards,
> Josh Wu
> >
> >
> > > + xlate->host_fmt = &isi_camera_formats[i];
> > > + xlate->code = code.code;
> > > + dev_dbg(icd->parent, "Providing format %s
> > > using code %d\n",
> > > + isi_camera_formats[0].name,
> > > code.code);
> > > + xlate++;
> > > + }
> > > }
> > > break;
> > > default:
> > > --
> > > 1.9.1
> > >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs
2015-10-19 1:48 ` Guennadi Liakhovetski
@ 2015-10-19 2:45 ` Josh Wu
0 siblings, 0 replies; 18+ messages in thread
From: Josh Wu @ 2015-10-19 2:45 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Dear Guennadi,
On 10/19/2015 9:48 AM, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> On Wed, 14 Oct 2015, Josh Wu wrote:
>
>> Hi, Dear Guennadi
>>
>> Thanks for the review.
>>
>> On 10/5/2015 12:43 AM, Guennadi Liakhovetski wrote:
>>> Hi Josh,
>>>
>>> On Tue, 22 Sep 2015, Josh Wu wrote:
>>>
>>>> we need to configure the YCC_SWAP bits in ISI_CFG2 according to current
>>>> sensor output and Atmel ISI output format.
>>>>
>>>> Current there are two cases Atmel ISI supported:
>>>> 1. Atmel ISI outputs YUYV format.
>>>> This case we need to setup YCC_SWAP according to sensor output
>>>> format.
>>>> 2. Atmel ISI output a pass-through formats, which means no swap.
>>>> Just setup YCC_SWAP as default with no swap.
>>>>
>>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>>> ---
>>>>
>>>> drivers/media/platform/soc_camera/atmel-isi.c | 43
>>>> ++++++++++++++++++++-------
>>>> 1 file changed, 33 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
>>>> b/drivers/media/platform/soc_camera/atmel-isi.c
>>>> index 45e304a..df64294 100644
>>>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>>>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>>>> @@ -103,13 +103,41 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
>>>> return readl(isi->regs + reg);
>>>> }
>>>> +static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
>>>> + const struct soc_camera_format_xlate *xlate)
>>>> +{
>>> This function will be called only for the four media codes from the
>>> switch-case statement below, namely for
>>>
>>> MEDIA_BUS_FMT_VYUY8_2X8
>>> MEDIA_BUS_FMT_UYVY8_2X8
>>> MEDIA_BUS_FMT_YVYU8_2X8
>>> MEDIA_BUS_FMT_YUYV8_2X8
>>>
>>>> + /* By default, no swap for the codec path of Atmel ISI. So codec
>>>> + * output is same as sensor's output.
>>>> + * For instance, if sensor's output is YUYV, then codec outputs YUYV.
>>>> + * And if sensor's output is UYVY, then codec outputs UYVY.
>>>> + */
>>>> + u32 cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_DEFAULT;
>>> Then this ISI_CFG2_YCC_SWAP_DEFAULT will only hold for
>>> MEDIA_BUS_FMT_YUYV8_2X8? Why don't you just add one more case below? Don't
>>> think this initialisation is really justified.
>> This default initial value is for all host_fmt_fourcc case. Not just for
>> V4L2_PIX_FMT_YUYV this case.
> Right, yes, missed this. How about this then:
>
> if (xlate->host_fmt->fourcc != V4L2_PIX_FMT_YUYV)
> return ISI_CFG2_YCC_SWAP_DEFAULT;
>
> switch (xlate->code) {
> case MEDIA_BUS_FMT_VYUY8_2X8:
> return ISI_CFG2_YCC_SWAP_MODE_3;
> case MEDIA_BUS_FMT_UYVY8_2X8:
> return ISI_CFG2_YCC_SWAP_MODE_2;
> case MEDIA_BUS_FMT_YVYU8_2X8:
> return ISI_CFG2_YCC_SWAP_MODE_1;
> }
>
> return ISI_CFG2_YCC_SWAP_DEFAULT;
>
> Or even exactly as you have it in your patch only remove the cfg2_yuv_swap
> variable and do "return" directly after each "case MEDIA_..." and one more
> with SWAP_DEFAULT in the end.
Right, that's exactly what I want to do. That looks very neat. Thanks
for your advice.
I'll fix it in v2.
>
>>>> +
>>>> + if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
>>>> + /* all convert to YUYV */
>>>> + switch (xlate->code) {
>>>> + case MEDIA_BUS_FMT_VYUY8_2X8:
>>>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_3;
>>>> + break;
>>>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>>>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_2;
>>>> + break;
>>>> + case MEDIA_BUS_FMT_YVYU8_2X8:
>>>> + cfg2_yuv_swap = ISI_CFG2_YCC_SWAP_MODE_1;
>>>> + break;
>>>> + }
>>>> + }
>>>> +
>>>> + return cfg2_yuv_swap;
>>>> +}
>>>> +
>>>> static void configure_geometry(struct atmel_isi *isi, u32 width,
>>>> - u32 height, u32 code)
>>>> + u32 height, const struct soc_camera_format_xlate *xlate)
>>>> {
>>>> u32 cfg2;
>>>> /* According to sensor's output format to set cfg2 */
>>>> - switch (code) {
>>>> + switch (xlate->code) {
>>>> default:
>>>> /* Grey */
>>>> case MEDIA_BUS_FMT_Y8_1X8:
>>>> @@ -117,16 +145,11 @@ static void configure_geometry(struct atmel_isi
>>>> *isi, u32 width,
>>>> break;
>>>> /* YUV */
>>>> case MEDIA_BUS_FMT_VYUY8_2X8:
>>>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_3 | ISI_CFG2_COL_SPACE_YCbCr;
>>>> - break;
>>>> case MEDIA_BUS_FMT_UYVY8_2X8:
>>>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_2 | ISI_CFG2_COL_SPACE_YCbCr;
>>>> - break;
>>>> case MEDIA_BUS_FMT_YVYU8_2X8:
>>>> - cfg2 = ISI_CFG2_YCC_SWAP_MODE_1 | ISI_CFG2_COL_SPACE_YCbCr;
>>>> - break;
>>>> case MEDIA_BUS_FMT_YUYV8_2X8:
>>>> - cfg2 = ISI_CFG2_YCC_SWAP_DEFAULT | ISI_CFG2_COL_SPACE_YCbCr;
>>>> + cfg2 = ISI_CFG2_COL_SPACE_YCbCr |
>>>> + setup_cfg2_yuv_swap(isi, xlate);
>>>> break;
>>>> /* RGB, TODO */
>>>> }
>>> I would move this switch-case completely inside setup_cfg2_yuv_swap().
>>> Just do
>>>
>>> cfg2 = setup_cfg2_yuv_swap(isi, xlate);
>>>
>>> and handle the
>>>
>>> case MEDIA_BUS_FMT_Y8_1X8:
>>>
>>> in the function too. These two switch-case statements really look
>>> redundant.
>> Technically, you can do that. But for my point of view, the
>> setup_cfg2_yuv_swap() only need to setup the yuv swap register field.
>>
>> And other cfg2 field need to be configured as well, especially in the case
>> sensor output a RGB data. That should be implement soon.
> Understand. I don't like redundant code and this certainly looks redundant
> to me. I'd be glad if you could remove this redundancy, but if you feel
> strongly about it, feel free to keep as is.
Good, thanks.
Best Regards,
Josh Wu
>
> Thanks
> Guennadi
>
>>>> @@ -407,7 +430,7 @@ static int start_streaming(struct vb2_queue *vq,
>>>> unsigned int count)
>>>> isi_writel(isi, ISI_INTDIS, (u32)~0UL);
>>>> configure_geometry(isi, icd->user_width, icd->user_height,
>>>> - icd->current_fmt->code);
>>>> + icd->current_fmt);
>>>> spin_lock_irq(&isi->lock);
>>>> /* Clear any pending interrupt */
>>>> --
>>>> 1.9.1
>>>>
>>> Thanks
>>> Guennadi
>> Best Regards,
>> Josh Wu
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats
2015-10-19 2:03 ` Guennadi Liakhovetski
@ 2015-10-19 2:50 ` Josh Wu
0 siblings, 0 replies; 18+ messages in thread
From: Josh Wu @ 2015-10-19 2:50 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Linux Media Mailing List, linux-arm-kernel, Laurent Pinchart
Dear Guennadi,
On 10/19/2015 10:03 AM, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> On Wed, 14 Oct 2015, Josh Wu wrote:
>
>> Dear Guennadi,
>>
>> Thanks for the review.
>>
>> On 10/5/2015 1:02 AM, Guennadi Liakhovetski wrote:
>>> On Tue, 22 Sep 2015, Josh Wu wrote:
>>>
>>>> This patch enable Atmel ISI preview path to convert the YUV to RGB format.
>>>>
>>>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>>>> ---
>>>>
>>>> drivers/media/platform/soc_camera/atmel-isi.c | 38
>>>> ++++++++++++++++++++-------
>>>> 1 file changed, 29 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
>>>> b/drivers/media/platform/soc_camera/atmel-isi.c
>>>> index e87d354..e33a16a 100644
>>>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>>>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>>>> @@ -201,13 +201,20 @@ static bool is_supported(struct soc_camera_device
>>>> *icd,
>>>> case V4L2_PIX_FMT_UYVY:
>>>> case V4L2_PIX_FMT_YVYU:
>>>> case V4L2_PIX_FMT_VYUY:
>>>> + /* RGB */
>>>> + case V4L2_PIX_FMT_RGB565:
>>>> return true;
>>>> - /* RGB, TODO */
>>>> default:
>>>> return false;
>>>> }
>>>> }
>>>> +static bool is_output_rgb(const struct soc_mbus_pixelfmt *host_fmt)
>>>> +{
>>>> + return host_fmt->fourcc == V4L2_PIX_FMT_RGB565 ||
>>>> + host_fmt->fourcc == V4L2_PIX_FMT_RGB32;
>>>> +}
>>>> +
>>> Why not just pass fourcc to this function? Or maybe just embed it in
>>> start_streaming - it won't clutter it a lot.
>> I think pass fourcc to the function is good.
>> Since configure_geometry() is hardware related, and the enable_preview_path is
>> also hardware related, so I prefer initialize enable_preview_path in
>> configure_geometry().
> But you don't, you do it in start_streaming() below.
Right, then I'll move it to configure_geometry() in v2..
> But actually my
> comment was not about _where_ to do it, but whether this calculation is
> worth a separate function. I would just perform this calculation directly
> where you're calling it:
>
> static ... start_streaming(...)
> {
> ...
> u32 fourcc = icd->current_fmt->host_fmt->fourcc;
>
> isi->enable_preview_path = fourcc == V4L2_PIX_FMT_RGB565 ||
> fourcc == V4L2_PIX_FMT_RGB32;
I thought this "function" might be called in other place, but actually
no one call it.
So yes, I think there is no need to have such separated function. I'll
fix it in v2. Thanks.
Best Regards,
Josh Wu
>
> Thanks
> Guennadi
>
>>>> static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
>>>> {
>>>> if (isi->active) {
>>>> @@ -467,6 +474,8 @@ static int start_streaming(struct vb2_queue *vq,
>>>> unsigned int count)
>>>> struct atmel_isi *isi = ici->priv;
>>>> int ret;
>>>> + isi->enable_preview_path = is_output_rgb(icd->current_fmt->host_fmt);
>>>> +
>>>> pm_runtime_get_sync(ici->v4l2_dev.dev);
>>>> /* Reset ISI */
>>>> @@ -688,6 +697,14 @@ static const struct soc_mbus_pixelfmt
>>>> isi_camera_formats[] = {
>>>> .order = SOC_MBUS_ORDER_LE,
>>>> .layout = SOC_MBUS_LAYOUT_PACKED,
>>>> },
>>>> + {
>>>> + .fourcc = V4L2_PIX_FMT_RGB565,
>>>> + .name = "RGB565",
>>>> + .bits_per_sample = 8,
>>>> + .packing = SOC_MBUS_PACKING_2X8_PADHI,
>>>> + .order = SOC_MBUS_ORDER_LE,
>>>> + .layout = SOC_MBUS_LAYOUT_PACKED,
>>>> + },
>>>> };
>>>> /* This will be corrected as we get more formats */
>>>> @@ -744,7 +761,7 @@ static int isi_camera_get_formats(struct
>>>> soc_camera_device *icd,
>>>> struct soc_camera_format_xlate *xlate)
>>>> {
>>>> struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>>>> - int formats = 0, ret;
>>>> + int formats = 0, ret, i, n;
>>>> /* sensor format */
>>>> struct v4l2_subdev_mbus_code_enum code = {
>>>> .which = V4L2_SUBDEV_FORMAT_ACTIVE,
>>>> @@ -778,13 +795,16 @@ static int isi_camera_get_formats(struct
>>>> soc_camera_device *icd,
>>>> case MEDIA_BUS_FMT_VYUY8_2X8:
>>>> case MEDIA_BUS_FMT_YUYV8_2X8:
>>>> case MEDIA_BUS_FMT_YVYU8_2X8:
>>>> - formats++;
>>>> - if (xlate) {
>>>> - xlate->host_fmt = &isi_camera_formats[0];
>>>> - xlate->code = code.code;
>>>> - xlate++;
>>>> - dev_dbg(icd->parent, "Providing format %s using code
>>>> %d\n",
>>>> - isi_camera_formats[0].name, code.code);
>>>> + n = ARRAY_SIZE(isi_camera_formats);
>>>> + formats += n;
>>>> + for (i = 0; i < n; i++) {
>>>> + if (xlate) {
>>> I'd put if outside of the loop, or just do
>>>
>>> + for (i = 0; xlate && i < n; i++) {
>> yes, that simpler one. I'll take it. Thanks.
>>
>> Best Regards,
>> Josh Wu
>>>
>>>> + xlate->host_fmt = &isi_camera_formats[i];
>>>> + xlate->code = code.code;
>>>> + dev_dbg(icd->parent, "Providing format %s
>>>> using code %d\n",
>>>> + isi_camera_formats[0].name,
>>>> code.code);
>>>> + xlate++;
>>>> + }
>>>> }
>>>> break;
>>>> default:
>>>> --
>>>> 1.9.1
>>>>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-10-19 2:51 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-22 5:14 [PATCH 0/5] media: atmel-isi: enable preview path to output RGB565 format Josh Wu
2015-09-22 5:14 ` [PATCH 1/5] media: atmel-isi: correct yuv swap according to different sensor outputs Josh Wu
2015-10-04 16:43 ` Guennadi Liakhovetski
2015-10-04 17:04 ` Guennadi Liakhovetski
2015-10-14 6:46 ` Josh Wu
2015-10-14 6:43 ` Josh Wu
2015-10-19 1:48 ` Guennadi Liakhovetski
2015-10-19 2:45 ` Josh Wu
2015-09-22 5:14 ` [PATCH 2/5] media: atmel-isi: prepare for the support of preview path Josh Wu
2015-09-22 5:14 ` [PATCH 3/5] media: atmel-isi: add code to setup correct resolution for " Josh Wu
2015-09-22 5:14 ` [PATCH 4/5] media: atmel-isi: setup YCC_SWAP correctly when using " Josh Wu
2015-10-04 16:50 ` Guennadi Liakhovetski
2015-10-14 6:44 ` Josh Wu
2015-09-22 5:14 ` [PATCH 5/5] media: atmel-isi: support RGB565 output when sensor output YUV formats Josh Wu
2015-10-04 17:02 ` Guennadi Liakhovetski
2015-10-14 6:57 ` Josh Wu
2015-10-19 2:03 ` Guennadi Liakhovetski
2015-10-19 2:50 ` Josh Wu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).