linux-kernel - Re: [PATCH v3 2/2] Added Digiteq Automotive MGB4 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d1b241b3-73ca-5069-2542-1b44c6e37d7f@gpxsee.org>
Date:   Mon, 7 Nov 2022 18:58:17 +0100
From:   Martin Tůma <tumic@...see.org>
To:     Hans Verkuil <hverkuil@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Michal Simek <michal.simek@...inx.com>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-i2c@...r.kernel.org, Lizhi Hou <lizhi.hou@....com>,
        Martin Tůma <martin.tuma@...iteqautomotive.com>
Subject: Re: [PATCH v3 2/2] Added Digiteq Automotive MGB4 driver

On 07. 11. 22 13:44, Hans Verkuil wrote:
>>>>>
>>>>> Same as for input: opening a video device appears to have a side-effect.
>>>>>
>>>>
>>>> The same reason as in the input case. You can have different displays with completely different video signals (even resolutions) connected to the output at any arbitrary time.
>>>
>>> It's similar to the input side: use the DV timings ioctls. Except there won't be a
>>> QUERY_DV_TIMINGS, since it is an output.
>>>
>>> But how do you know what the display supports? Is it using an EDID? In that case you
>>> need to support VIDIOC_G_EDID as well, and probably some of the DV controls:
>>>
>>> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/ext-ctrls-dv.html
>>>
>>
>> There is no EDID or another type of autoconfiguration. In the cars, there is simply always the right display connected to the radio (infotainment system) and the values are hardcoded on both sides.
>>
>> In the mgb4 card (that must support arbitrary size displays) we solve this by the sysfs configuration - see the "Common FPDL3/GMSL output parameters" in the documentation. You set the display
>> properties in sysfs (usualy using some udev rules at boot time) and the v4l2 part of the driver then uses this values.
> 
> So for output at least the values do not 'just' change, it is something that is configured
> via sysfs. So the driver knows when this happens, and G_DV_TIMINGS would just return that
> configuration. You can choose (if it would make sense) to support S_DV_TIMINGS to set the
> configuration instead of sysfs, but I don't know if that would be sufficient or if it
> even makes sense.
> 
> Changing timings when someone is streaming is also something you need to think about:
> you can either refuse changing timings (return EBUSY), or accepts it, but then the
> driver has to call vb2_queue_error() and raise the V4L2_EVENT_SOURCE_CHANGE event to
> signal that the format changed and userspace has to take action.
> 

Yes, the output case is simpler as the source of the events can be 
controlled.

Changing timings (or resolution) when someone is streaming is not 
allowed, there are checks in the sysfs properties code that do not allow 
the change (you get EBUSY when writing to sysfs when the device is open).

In general I want to let all the HW setup to happen in sysfs as there 
are a lot of properties that are not a standard part of v4l2 and I want 
the configuration to happen on a single place, not something in sysfs 
and something else in v4l2. Moreover, the properties must be set 
correctly to get a working input/output stream so those properties are 
not some "options" as usual in the v4l2 controls.

M.