On 05/22/2017 11:02 AM, Hans Verkuil wrote:
>> --- a/drivers/media/platform/s3c-camif/camif-regs.c
>> +++ b/drivers/media/platform/s3c-camif/camif-regs.c
>> @@ -58,7 +58,7 @@ void camif_hw_set_test_pattern(struct camif_dev *camif,
>> unsigned int pattern)
>> }
>>
>> void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
>> - unsigned int cr, unsigned int cb)
>> + unsigned int cb, unsigned int cr)
>> {
>> static const struct v4l2_control colorfx[] = {
>> { V4L2_COLORFX_NONE, CIIMGEFF_FIN_BYPASS },
>
> This will also affect this line:
>
> cfg |= cr | (cb << 13);
>
> cr and cb are now swapped so this will result in a different color.
>
> Sylwester, who is wrong here: the prototype or how this function is called?
>
> I suspect that Gustavo is right and that the prototype is wrong. But in that
> case this patch should also change the cfg assignment.
The function is currently called in a wrong way, it's clear from looking
at the prototype. CR should end up in bits 0:7 and CR in bits 20:13 of
the register. So yes, colour will change after applying the patch - to the
expected one, matching the user API documentation.
Unfortunately I can't test it because I have only the s3c2440 SoC based
evaluation board where the image effect is not supported.
Probably a more straightforward fix would be to amend the callers (apologies
Gustavo for misleading suggestions). But I'm inclined to apply the $subject
patch as is to just close this bug report case.
--
Regards,
Sylwester
