Laurent Pinchart wrote:
> Hi Sakari,
Hi Laurent and Michael!
...
>>> + return in_info->bpp - out_info->bpp + additional_shift <= 6;
>>
>> Currently there are no formats that would behave badly in this check?
>> Perhaps it'd be good idea to take that into consideration. The shift
>> that can be done is even.
>
> I've asked Michael to remove the check because we have no misbehaving formats
> :-) Do you think we need to add a check back ?
I think it would be helpful in debugging if someone decides to attach a
sensor which supports a shift of non-even bits (8 and 9 bits, for
example). In any case an invalid configuration is possible in such case,
and I don't think that should be allowed, should it?
>>> @@ -247,6 +296,7 @@ static int isp_video_validate_pipeline(struct
>>> isp_pipeline *pipe)
>>>
>>> return -EPIPE;
>>>
>>> while (1) {
>>>
>>> + unsigned int link_has_shifter;
>>
>> link_has_shifter is only used in one place. Would it be cleaner to test
>> below if it's the CCDC? A comment there could be nice, too.
>
> I would like that better as well, but between the line where link_has_shifter
> is set and the line where it is checked, the subdev variable changes so we
> can't just check subdev == &isp->isp_ccdc.subdev there.
That's definitely valid. I take my comment back. The variable could be
called is_ccdc, though, since only the CCDC has that feature. No need to
generalise. :-)
Cheers,
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
