Re: [PATCH] media: bcm2835-unicam: Fix pipeline wrong validation for unpacked formats

[Eugen Hristev]

On 6/3/26 09:48, Jai Luthra wrote:
> Hi Eugen,
> 
> Quoting Eugen Hristev (2026-05-20 21:07:00)
>> The commit
>> 08f9794d9b79 ("media: bcm2835-unicam: Fix RGB format / mbus code association")
>> introduced a check to see whether the format requested is the same as the
>> fourcc in the format list.
>>
>> However, this breaks the case when userspace requested an unpacked fourcc,
>> e.g. RG10.
>>
>> Unicam can work with or without unpacking pixels, e.g. pRAA or RG10, depending
>> on what userspace requests.
>> In the unpacking case, a dedicated register is being set.
>>
>> If the userspace requests pRAA, this works, because the check validates the
>> pipeline:
>>
>> v4l2-ctl -d /dev/video0 --set-fmt-video=width=3280,height=2464,pixelformat=pRAA \
>>  --stream-mmap --stream-count=1 --stream-to=frame.raw
>>
>> but, with
>> v4l2-ctl -d /dev/video0 --set-fmt-video=width=3280,height=2464,pixelformat=RG10 \
>> --stream-mmap --stream-count=1 --stream-to=frame.raw
>>
>> unicam complains at validation level:
>>
>> image: format mismatch: 0x300f <=> RG10 little-endian (0x30314752)
>>
>> This should work, because MEDIA_BUS_FMT_SRGGB10_1X10 can be packed into either
>> RG10 or pRAA depending on the packing register.
>>
>> To fix this, modified the condition check to also allow in the case when
>> requested format (fmt->pixelformat) is equal to fmtinfo->unpacked_fourcc.
>>
>> Fixes: 08f9794d9b79 ("media: bcm2835-unicam: Fix RGB format / mbus code association")
>> Signed-off-by: Eugen Hristev <ehristev@xxxxxxxxxx>
>> ---
>>  drivers/media/platform/broadcom/bcm2835-unicam.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
>> index 8d28ba0b59a3..cc7627e9a51a 100644
>> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
>> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
>> @@ -2158,7 +2158,8 @@ static int unicam_video_link_validate(struct media_link *link)
>>                  * In order to allow the applications using the old behaviour to
>>                  * run, let's accept the old combination, but warn about it.
>>                  */
>> -               if (fmtinfo->fourcc != fmt->pixelformat) {
>> +               if (fmt->pixelformat != fmtinfo->fourcc &&
>> +                   fmt->pixelformat != fmtinfo->unpacked_fourcc) {
>>                         if ((fmt->pixelformat == V4L2_PIX_FMT_BGR24 &&
>>                              format->code == MEDIA_BUS_FMT_BGR888_1X24) ||
>>                             (fmt->pixelformat == V4L2_PIX_FMT_RGB24 &&
>>
> 
> Reviewed-by: Jai Luthra <jai.luthra@xxxxxxxxxxxxxxxx>
> 

Hi Sakari,

I saw you taking some of the patches on unicam, this one is the most
important as all unpacked formats are basically broken for a very long
time (months) and the patch is pending for quite some time.

This should also go in stable as some older versions also affected.

Thanks,
Eugen

> Thanks,
> Jai
> 
>> ---
>> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
>> change-id: 20260520-bcmpi-2c4850314e21
>>
>> Best regards,
>> --  
>> Eugen Hristev <ehristev@xxxxxxxxxx>
>>
>>