linux-kernel - Re: [PATCH v5 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <93a7aa80-52c5-86c4-c264-37849b467dd3@synopsys.com>
Date:   Tue, 4 Jul 2017 13:33:50 +0100
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     Hans Verkuil <hansverk@...co.com>,
        Jose Abreu <Jose.Abreu@...opsys.com>,
        Hans Verkuil <hverkuil@...all.nl>,
        <linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     Carlos Palminha <CARLOS.PALMINHA@...opsys.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        "Sylwester Nawrocki" <snawrocki@...nel.org>
Subject: Re: [PATCH v5 2/4] [media] platform: Add Synopsys Designware HDMI RX
 Controller Driver



On 04-07-2017 10:39, Hans Verkuil wrote:
>
>>>>>> +static const struct v4l2_subdev_video_ops
>>>>>> dw_hdmi_sd_video_ops = {
>>>>>> +    .s_routing = dw_hdmi_s_routing,
>>>>>> +    .g_input_status = dw_hdmi_g_input_status,
>>>>>> +    .g_parm = dw_hdmi_g_parm,
>>>>>> +    .g_dv_timings = dw_hdmi_g_dv_timings,
>>>>>> +    .query_dv_timings = dw_hdmi_query_dv_timings,
>>>>> No s_dv_timings???
>>>> Hmm, yeah, I didn't implement it because the callchain and the
>>>> player I use just use {get/set}_fmt. s_dv_timings can just
>>>> populate the fields and replace them with the detected dv_timings
>>>> ? Just like set_fmt does? Because the controller has no scaler.
>>> No, s_dv_timings is the function that actually sets
>>> dw_dev->timings.
>>> After you check that it is valid of course (call
>>> v4l2_valid_dv_timings).
>>>
>>> set_fmt calls get_fmt which returns the information from
>>> dw_dev->timings.
>>>
>>> But it is s_dv_timings that has to set dw_dev->timings.
>>>
>>> With the current code you can only capture 640x480 (the default
>>> timings).
>>> Have you ever tested this with any other timings? I don't quite
>>> understand
>>> how you test.
>> I use mpv to test with a wrapper driver that just calls the
>> subdev ops and sets up a video dma.
>>
>> Ah, I see now. I failed to port the correct callbacks and in the
>> upstream version I'm using I only tested with 640x480 ...
>>
>> But apart from that this is a capture device without scaling so I
>> can not set timings, I can only return them so that applications
>> know which format I'm receiving, right? So my s_dv_timings will
>> return the same as query_dv_timings ...
> Well, to be precise: s_dv_timings just accepts what the application
> gives it (as long as it is within the dv_timings capabilities). But
> those timings come in practice from a query_dv_timings call from the
> application.
>
> The core rule is that receivers cannot randomly change timings since
> timings are related to buffer sizes. You do not want the application
> to allocate buffers for 640x480 and when the source changes to 1920x1080
> have those buffers suddenly overflow.
>
> Instead the app queries the timings, allocates the buffers, start
> streaming and when the timings change it will get an event so it can
> stop streaming, reallocate buffers, and start the process again.
>
> In other words, the application is in control here.
>

... But this is not true for mpv/mplayer. They first try to set a
default format (by using s_fmt) and then query the format again
(by using g_fmt) ... So dv_timings are never used. Are these apps
broken? Im only using them because of performance, do you
recommend others?

Best regards,
Jose Miguel Abreu