Message-ID: <CAJ+vNU267VyOiq+fyv0dRm4Mui3YfK-ybqeOSENGMX6LmzZdcQ@mail.gmail.com>
Date:   Mon, 6 Nov 2017 22:04:29 -0800
From:   Tim Harvey <tharvey@...eworks.com>
To:     Hans Verkuil <hverkuil@...all.nl>
Cc:     linux-media <linux-media@...r.kernel.org>,
        alsa-devel@...a-project.org,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Steve Longerbeam <slongerbeam@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Hans Verkuil <hansverk@...co.com>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>
Subject: Re: [PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver

On Fri, Oct 20, 2017 at 7:00 AM, Tim Harvey <tharvey@...eworks.com> wrote:
> On Thu, Oct 19, 2017 at 12:39 AM, Hans Verkuil <hverkuil@...all.nl> wrote:
> <snip>
>>>
>>> Regarding video standard detection where this chip provides me with
>>> vertical-period, horizontal-period, and horizontal-pulse-width I
>>> should be able to detect the standard simply based off of
>>> vertical-period (framerate) and horizontal-period (line width
>>> including blanking) right? I wasn't sure if my method of matching
>>> these within 14% tolerance made sense. I will be removing the hsmatch
>>> logic from that as it seems the horizontal-pulse-width should be
>>> irrelevant.
>>
>> For proper video detection you ideally need:
>>
>> h/v sync size
>> h/v back/front porch size
>> h/v polarity
>> pixelclock (usually an approximation)
>>
>> The v4l2_find_dv_timings_cap() helper can help you find the corresponding
>> timings, allowing for pixelclock variation.
>>
>> That function assumes that the sync/back/frontporch values are all known.
>> But not all devices can actually discover those values. What can your
>> hardware detect? Can it tell front and backporch apart? Can it determine
>> the sync size?
>>
>> I've been considering for some time to improve that helper function to be
>> able to handle hardware that isn't able separate sync/back/frontporch values.
>>
>
> The TDA1997x provides only the vertical/horizontal periods and the
> horizontal pulse
> width (section 8.3.4 of datasheet [1]).
>
> Can you point me to a good primer on the relationship between these
> values and the h/v back/front porch?
>
> Currently I iterate over the list of known formats calculating hper
> (bt->pixelclock / V4L2_DV_BT_FRAME_WIDTH(bt)) and vper (hper /
> V4L2_DV_BT_FRAME_HEIGHT(bt)) (framerate) and find the closest match
> within +/- 7% tolerance. The list of supported formats is sorted by
> framerate then width.
>
>         /* look for matching timings */
>         for (i = 0; i < ARRAY_SIZE(tda1997x_hdmi_modes); i++) {
>                 const struct tda1997x_video_std *std = &tda1997x_hdmi_modes[i];
>                 const struct v4l2_bt_timings *bt = &std->timings.bt;
>                 int _hper, _vper, _hsper;
>                 int vmin, vmax, hmin, hmax, hsmin, hsmax;
>                 int vmatch, hsmatch;
>
>                 width = V4L2_DV_BT_FRAME_WIDTH(bt);
>                 lines = V4L2_DV_BT_FRAME_HEIGHT(bt);
>
>                 _hper = (int)bt->pixelclock / (int)width;
>                 _vper = _hper / lines;
>                 _hsper = (int)bt->pixelclock / (int)bt->hsync;
>                 if (bt->interlaced)
>                         _vper *= 2;
>                 /* vper +/- 0.7% */
>                 vmin = 993 * (27000000 / _vper) / 1000;
>                 vmax = 1007 * (27000000 / _vper) / 1000;
>                 _hsper = (int)bt->pixelclock / (int)bt->hsync;
>                 if (bt->interlaced)
>                         _vper *= 2;
>                 /* vper +/- 0.7% */
>                 vmin = 993 * (27000000 / _vper) / 1000;
>                 vmax = 1007 * (27000000 / _vper) / 1000;
>                 /* hper +/- 0.7% */
>                 hmin = 993 * (27000000 / _hper) / 1000;
>                 hmax = 1007 * (27000000 / _hper) / 1000;
>
>                 /* vmatch matches the framerate */
>                 vmatch = ((vper <= vmax) && (vper >= vmin)) ? 1 : 0;
>                 /* hmatch matches the width */
>                 hmatch = ((hper <= hmax) && (hper >= hmin)) ? 1 : 0;
>                 if (vmatch && hsmatch) {
>                         v4l_info(state->client,
>                                  "resolution: %dx%d%c@%d (%d/%d/%d) %dMHz %d\n",
>                                  bt->width, bt->height, bt->interlaced?'i':'p',
>                                  _vper, vper, hper, hsper, pixrate, hsmatch);
>                         state->fps = (int)bt->pixelclock / (width * lines);
>                         state->std = std;
>                         return 0;
>                 }
>         }
>
> Note that I've thrown out any comparisons based on horizontal pulse
> width from my first patch as that didn't appear to fit well. So far
> the above works well however I do fail to recognize the following
> modes (using a Marshall SG4K HDMI test generator):
>

Hans,

I've found that I do indeed need to look at the 'hsper' that the TDA
provides above along with the vper/hper as there are several timings
that match a given vper/hper. However I haven't figured out how to
make sense of the hsper value that is returned.

Here are some example timings and the vper/hper/hsper returned from the TDA:
V4L2_DV_BT_DMT_1280X960P60 449981/448/55
V4L2_DV_BT_DMT_1280X1024P60 449833/420/27
V4L2_DV_BT_DMT_1280X768P60 450021/568/11
V4L2_DV_BT_DMT_1360X768P60 449867/564/34

Do you know what the hsper could be here? It doesn't appear to match
v4l2_bt_timings hsync ((27MHz/bt->pixelclock)*bt->hsync).

Thanks,

Tim

Powered by blists - more mailing lists