Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <72494a61-5be8-033b-5bcd-59699a226002@xs4all.nl>
Date:   Tue, 13 Jun 2023 14:18:32 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [RESEND PATCH v6 1/1] Added Digiteq Automotive MGB4 driver
Content-Language: en-US
To:     =?UTF-8?Q?Martin_T=c5=afma?= <tumic@gpxsee.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>
Cc:     linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        =?UTF-8?Q?Martin_T=c5=afma?= <martin.tuma@digiteqautomotive.com>
References: <20230524112126.2242-1-tumic@gpxsee.org>
 <20230524112126.2242-2-tumic@gpxsee.org>
 <3a7da3cd-8d03-a2c4-0534-a75565aefc13@xs4all.nl>
 <7072a8f3-5c9e-1170-e480-6fb57b95110f@gpxsee.org>
 <6b792de3-bb2c-d2b5-a652-eca6d20dad20@xs4all.nl>
 <c34db414-159a-313f-90eb-2bfc0f4496fa@gpxsee.org>
 <089e728b-0596-d3e3-39a1-651a3ac73e33@xs4all.nl>
 <f72a7380-d8bc-24bf-630c-75f8ffd6abf3@gpxsee.org>
From:   Hans Verkuil <hverkuil-cisco@xs4all.nl>
In-Reply-To: <f72a7380-d8bc-24bf-630c-75f8ffd6abf3@gpxsee.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Martin,

On 12/06/2023 13:34, Martin Tůma wrote:
> On 12. 06. 23 10:51, Hans Verkuil wrote:
>> On 08/06/2023 17:30, Martin Tůma wrote:
>>> On 08. 06. 23 12:23, Hans Verkuil wrote:
>>>
>>>> Can you make a list of which sysfs properties correspond to existing V4L2
>>>> format or timing fields and which are 'new'?
>>>>
>>>
>>> On the left all the current mgb4 sysfs properties (see the admin-guide doc from the patch for description), on the right v4l2 structures where they could be mapped (may not be true for all of them in
>>> the patch, I will check it and update the code in v7)
>>>
>>>
>>> --- PCIE CARD ---
>>>
>>> module_type        -
>>> module_version        -
>>> fw_type            -
>>> fw_version        -
>>> serial_number        -
>>> temperature        hwmon
>>>
>>> --- INPUTS ---
>>>
>>> input_id        -
>>> oldi_lane_width        -
>>> color_mapping        -
>>> link_status        v4l2_input.status (V4L2_IN_ST_NO_SYNC)
>>> stream_status        v4l2_input.status (V4L2_IN_ST_NO_SIGNAL)
>>> video_width        v4l2_bt_timings.width
>>> video_height        v4l2_bt_timings.height
>>> vsync_status        v4l2_bt_timings.polarities
>>> hsync_status        v4l2_bt_timings.polarities
>>> vsync_gap_length    -
>>> hsync_gap_length    -
>>> pclk_frequency        v4l2_bt_timings.pixelclock
>>> hsync_width        v4l2_bt_timings.hsync
>>> vsync_width        v4l2_bt_timings.vsync
>>> hback_porch        v4l2_bt_timings.hbackporch
>>> hfront_porch        v4l2_bt_timings.hfrontporch
>>> vback_porch        v4l2_bt_timings.vbackporch
>>> vfront_porch        v4l2_bt_timings.vfrontporch
>>> frequency_range        -
>>> alignment        v4l2_pix_format.bytesperline
>>> fpdl3_input_width    -
>>> gmsl_mode        -
>>> gmsl_stream_id        -
>>> gmsl_fec        -
>>>
>>> --- OUTPUTS ---
>>>
>>> output_id        -
>>> video_source        -
>>> display_width        v4l2_bt_timings.width
>>> display_height        v4l2_bt_timings.height
>>> frame_rate        v4l2_frmivalenum
>>
>> The frame rate is a property of the width/height+blanking and the
>> pixel clock frequency. IMHO it does not make sense to have this as
>> a writable property. Read-only is OK.
>>
>>> hsync_polarity        v4l2_bt_timings.polarities
>>> vsync_polarity        v4l2_bt_timings.polarities
>>> de_polarity        -
>>> pclk_frequency        v4l2_bt_timings.pixelclock
>>> hsync_width        v4l2_bt_timings.hsync
>>> vsync_width        v4l2_bt_timings.vsync
>>> vsync_width        v4l2_bt_timings.vsync
>>> hback_porch        v4l2_bt_timings.hbackporch
>>> hfront_porch        v4l2_bt_timings.hfrontporch
>>> vback_porch        v4l2_bt_timings.vbackporch
>>> vfront_porch        v4l2_bt_timings.vfrontporch
>>> alignment        v4l2_pix_format.bytesperline
>>> fpdl3_output_width    -
>>>
>>>
>>> M.
>>
>> The property I am most concerned with is alignment (both for input and output).
>> But it is not clear to me what the use-case is.
>>
> 
> Hi,
> The use-case is to provide the alignment required by some video processing chips. We have a product based on NVIDIA Jetson TX2 that uses the mgb4 cards and the HW video encoding needs a specific
> alignment to work.

OK. I would suggest that for this property it has a default value of 0 (i.e. a 1 byte alignment),
and in that case VIDIOC_S_FMT allows userspace to set bytesperline to whatever they want. I.e.,
this is the normal behavior for DMA engines that can deal with custom padding at the end of each
line.

If it is > 0, then bytesperline is fixed, based on this value.

That way both methods are supported fairly cleanly.

BTW, what is missing in the property documentation for writable properties is what the default
value is. That must be documented as well.

Regards,

	Hans