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

Daniel Scally
2021-09-09T22:37:00Z

Hi Paul

On 14/08/2021 21:56, Laurent Pinchart wrote:
> Hi Daniel,
>
> On Fri, Aug 13, 2021 at 10:45:48AM +0100, Daniel Scally wrote:
>> On 13/08/2021 04:05, Laurent Pinchart wrote:
>>> 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.
> I think it's worth investigating this. The hblank computed here is
> clearly incorrect, and would thus be useless for all practical purposes.
> As usual with OmniVision, the datasheet is also quite useless.
>
> Paul, do you have any information about this ?

A gentle ping on this...I played around setting HTS / VTS values whilst
the camera was running windows; and it behaves as you'd expect it to
(raising/lowering the frame rate), so as far as I can tell the sensor
itself isn't doing anything unusual...

>>>>>> + 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,