On 6/2/20 7:52 AM, Dafna Hirschfeld wrote: > > > On 01.06.20 14:16, Kaaira Gupta wrote: >> On Fri, May 29, 2020 at 05:43:57PM +0200, Dafna Hirschfeld wrote: >>> Hi, >>> Thanks for the patch >>> >>> I don't know how real devices handle ARGB formats, >>> I wonder if it should be the part of the debayer. >> >> Hi! qcam tries to support BA24 as it is one of the formats that vimc >> lists as its supported formats wih --list-formats. Shouldn't BA24 be >> possible to capture with vimc? > > Hi, > Just to clarify, when listing the supported formats of a video node, the node lists > the formats that the video node as an independent media entity support. > It does not mean that the 'camera' as a whole (that is, the media topology graph) supports > all the formats that the video node lists. When interacting with a video node or > a subdevice node, one interacts only with that specific entity. > In the case of vimc, the RGB video node as an independent entity supports BA24 so the format > appears in the list of the its supported formats. But since the Debayer does not > support it, the format can not be generated by the entire vimc topology. > This is not a bug. This is also my understanding. You should have an -EPIPE error when start streaming though, it shouldn't fail silently. Regards, Helen > > Hope t was helpful, > Dafna > >> >> If yes, which entity should support it, if not debayer? Should there be >> a separate conversion entity, or should we keep the support in debayer >> itself for efficiency issues? >> >>> >>> >>> On 28.05.20 20:57, Kaaira Gupta wrote: >>>> Running qcam for pixelformat 0x34324142 showed that vimc debayer does >>>> not support it. Hence, add the support for Alpha (255). >>> >>> I would change the commit log to: >>> >>> Add support for V4L2_PIX_FMT_RGB24 format in the debayer >>> and set the alpha channel to constant 255. >>> >>> Thanks, >>> Dafna >>> >>>> >>>> Signed-off-by: Kaaira Gupta <kgupta@xxxxxxxxxxxxx> >>>> --- >>>> .../media/test-drivers/vimc/vimc-debayer.c | 27 ++++++++++++------- >>>> 1 file changed, 18 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/media/test-drivers/vimc/vimc-debayer.c b/drivers/media/test-drivers/vimc/vimc-debayer.c >>>> index c3f6fef34f68..f34148717a40 100644 >>>> --- a/drivers/media/test-drivers/vimc/vimc-debayer.c >>>> +++ b/drivers/media/test-drivers/vimc/vimc-debayer.c >>>> @@ -62,6 +62,7 @@ static const u32 vimc_deb_src_mbus_codes[] = { >>>> MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, >>>> MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA, >>>> MEDIA_BUS_FMT_RGB888_1X32_PADHI, >>>> + MEDIA_BUS_FMT_ARGB8888_1X32 >>>> }; >>>> static const struct vimc_deb_pix_map vimc_deb_pix_map_list[] = { >>>> @@ -322,15 +323,23 @@ static void vimc_deb_process_rgb_frame(struct vimc_deb_device *vdeb, >>>> unsigned int i, index; >>>> vpix = vimc_pix_map_by_code(vdeb->src_code); >>>> - index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 3); >>>> - for (i = 0; i < 3; i++) { >>>> - switch (vpix->pixelformat) { >>>> - case V4L2_PIX_FMT_RGB24: >>>> - vdeb->src_frame[index + i] = rgb[i]; >>>> - break; >>>> - case V4L2_PIX_FMT_BGR24: >>>> - vdeb->src_frame[index + i] = rgb[2 - i]; >>>> - break; >>>> + >>>> + if (vpix->pixelformat == V4L2_PIX_FMT_ARGB32) { >>>> + index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 4); >>>> + vdeb->src_frame[index] = 255; >>>> + for (i = 0; i < 3; i++) >>>> + vdeb->src_frame[index + i + 1] = rgb[i]; >>>> + } else { >>>> + index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 3); >>>> + for (i = 0; i < 3; i++) { >>>> + switch (vpix->pixelformat) { >>>> + case V4L2_PIX_FMT_RGB24: >>>> + vdeb->src_frame[index + i] = rgb[i]; >>>> + break; >>>> + case V4L2_PIX_FMT_BGR24: >>>> + vdeb->src_frame[index + i] = rgb[2 - i]; >>>> + break; >>>> + } >>>> } >>>> } >>>> } >>>>