Re: [PATCH v2 08/12] media: i2c: Add hblank control to ov8865

Daniel Scally
2021-08-13T09:45:56Z

Hi Laurent

On 13/08/2021 04:05, Laurent Pinchart wrote:
> Hi Daniel,
>
> On Tue, Aug 10, 2021 at 11:07:22PM +0100, Daniel Scally wrote:
>> On 10/08/2021 15:29, Sakari Ailus wrote:
>>> On Mon, Aug 09, 2021 at 11:58:41PM +0100, Daniel Scally wrote:
>>>> @@ -2542,6 +2544,13 @@ static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
>>>> 0, 0, ov8865_test_pattern_menu);
>>>>
>>>> /* Blanking */
>>>> + hblank = mode->hts < mode->output_size_x ? 0 : mode->hts - mode->output_size_x;
>>> Is the result in relation with the analogue crop size? Based on the above
>>> it wouldn't seem like that.
>> This was a weird one actually. My understanding was that HTS should
>> always be >= the horizontal crop plus hblank...but that doesn't hold
>> true for some of this driver's modes and nor does it hold true when
>> running the camera in Windows (I checked the registers whilst running
>> it). So I went with setting hblank to 0 if the mode's HTS exceeded the
>> horizontal crop as the only way I could see to reconcile that.
> There's something very fishy here, HTS is, by definition, equal to the
> analog crop width plus the horizontal blanking. I suspect that the
> values in ov8865_modes are wrong.

I thought that initially too but confirming that the same thing happened
running windows switched me into "you're probably wrong" mode. If we're
confident that the HTS is likely wrong though I can add an extra patch
to bring those into lining with that definition.

>
>>>> + ctrls->hblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_HBLANK, hblank,
>>>> + hblank, 1, hblank);
>>>> +
>>>> + if (ctrls->hblank)
>>>> + ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>>>> +
>>>> vblank_max = OV8865_TIMING_MAX_VTS - mode->output_size_y;
>>>> vblank_def = mode->vts - mode->output_size_y;
>>>> ctrls->vblank = v4l2_ctrl_new_std(handler, ops, V4L2_CID_VBLANK,