| Message ID | 20200515142952.20163-2-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: staging: rkisp1: various fixes to capture formats | expand |
Hi Dafna, On 5/15/20 11:29 AM, Dafna Hirschfeld wrote: > According to the TRM, the YUV->RGB conversion outputs > "24 bit word". What it means is that 4 bytes are used with > 24bit for the RGB and the last byte is ignored. > This matches format V4L2_PIX_FMT_XBGR32. I would just improve this a bit, since you only mention the number of bytes, but doesn't mention why the colors are swapped. My suggestion: According to the TRM, the YUV->RGB conversion outputs RGB 888 format with 4 bytes, where the last byte is ignored, using big endian representation:
Hi Dafna, On Fri, May 15, 2020 at 4:30 PM Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > According to the TRM, the YUV->RGB conversion outputs > "24 bit word". What it means is that 4 bytes are used with > 24bit for the RGB and the last byte is ignored. I don't see this mentioned in the datasheets I have. On the other hand, XBGR32 indeed makes much more sense, as the 3-byte RGB isn't a very popular format. Have you validated that the hardware behavior indeed matches that? Best regards, Tomasz > This matches format V4L2_PIX_FMT_XBGR32. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > drivers/staging/media/rkisp1/rkisp1-capture.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c > index f69235f82c45..61b9ebe577b2 100644 > --- a/drivers/staging/media/rkisp1/rkisp1-capture.c > +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c > @@ -276,7 +276,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = { > }, > /* rgb */ > { > - .fourcc = V4L2_PIX_FMT_RGB24, > + .fourcc = V4L2_PIX_FMT_XBGR32, > .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA, > .output_format = RKISP1_MI_CTRL_SP_OUTPUT_RGB888, > }, { > -- > 2.17.1 >
On 27.05.20 01:04, Tomasz Figa wrote: > Hi Dafna, > > On Fri, May 15, 2020 at 4:30 PM Dafna Hirschfeld > <dafna.hirschfeld@collabora.com> wrote: >> >> According to the TRM, the YUV->RGB conversion outputs >> "24 bit word". What it means is that 4 bytes are used with >> 24bit for the RGB and the last byte is ignored. > > I don't see this mentioned in the datasheets I have. On the other > hand, XBGR32 indeed makes much more sense, as the 3-byte RGB isn't a > very popular format. Have you validated that the hardware behavior > indeed matches that? Hi, yes I validated it, I also found it mentioned here: http://rockchip.fr/RK3288%20TRM/rk3288-chapter-31-image-signal-processing-(isp).pdf under section "31.3.9 YCbCr to RGB Conversion" Thanks, Dafna > > Best regards, > Tomasz > >> This matches format V4L2_PIX_FMT_XBGR32. >> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >> --- >> drivers/staging/media/rkisp1/rkisp1-capture.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c >> index f69235f82c45..61b9ebe577b2 100644 >> --- a/drivers/staging/media/rkisp1/rkisp1-capture.c >> +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c >> @@ -276,7 +276,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = { >> }, >> /* rgb */ >> { >> - .fourcc = V4L2_PIX_FMT_RGB24, >> + .fourcc = V4L2_PIX_FMT_XBGR32, >> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA, >> .output_format = RKISP1_MI_CTRL_SP_OUTPUT_RGB888, >> }, { >> -- >> 2.17.1 >>
On Wed, Jun 10, 2020 at 6:11 PM Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > > > On 27.05.20 01:04, Tomasz Figa wrote: > > Hi Dafna, > > > > On Fri, May 15, 2020 at 4:30 PM Dafna Hirschfeld > > <dafna.hirschfeld@collabora.com> wrote: > >> > >> According to the TRM, the YUV->RGB conversion outputs > >> "24 bit word". What it means is that 4 bytes are used with > >> 24bit for the RGB and the last byte is ignored. > > > > I don't see this mentioned in the datasheets I have. On the other > > hand, XBGR32 indeed makes much more sense, as the 3-byte RGB isn't a > > very popular format. Have you validated that the hardware behavior > > indeed matches that? > > Hi, yes I validated it, I also found it mentioned here: > > http://rockchip.fr/RK3288%20TRM/rk3288-chapter-31-image-signal-processing-(isp).pdf > > under section "31.3.9 YCbCr to RGB Conversion" Okay, great. Thanks! Feel free to add my Reviewed-by after addressing Helen's comments. Best regards, Tomasz
diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c index f69235f82c45..61b9ebe577b2 100644 --- a/drivers/staging/media/rkisp1/rkisp1-capture.c +++ b/drivers/staging/media/rkisp1/rkisp1-capture.c @@ -276,7 +276,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = { }, /* rgb */ { - .fourcc = V4L2_PIX_FMT_RGB24, + .fourcc = V4L2_PIX_FMT_XBGR32, .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA, .output_format = RKISP1_MI_CTRL_SP_OUTPUT_RGB888, }, {
According to the TRM, the YUV->RGB conversion outputs "24 bit word". What it means is that 4 bytes are used with 24bit for the RGB and the last byte is ignored. This matches format V4L2_PIX_FMT_XBGR32. Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> --- drivers/staging/media/rkisp1/rkisp1-capture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)