Hi Maxime & everyone,

Sorry for being inactive in the discussions about this patchset for the last
couple of weeks.

> +const static struct analog_parameters tv_modes_parameters[] = {
> +     TV_MODE_PARAMETER(DRM_MODE_ANALOG_NTSC,
> +                       NTSC_LINES_NUMBER,
> +                       NTSC_LINE_DURATION_NS,
> +                       PARAM_RANGE(NTSC_HACT_DURATION_MIN_NS,
> +                                   NTSC_HACT_DURATION_TYP_NS,
> +                                   NTSC_HACT_DURATION_MAX_NS),
> +                       PARAM_RANGE(NTSC_HFP_DURATION_MIN_NS,
> +                                   NTSC_HFP_DURATION_TYP_NS,
> +                                   NTSC_HFP_DURATION_MAX_NS),
> +                       PARAM_RANGE(NTSC_HSLEN_DURATION_MIN_NS,
> +                                   NTSC_HSLEN_DURATION_TYP_NS,
> +                                   NTSC_HSLEN_DURATION_MAX_NS),
> +                       PARAM_RANGE(NTSC_HBP_DURATION_MIN_NS,
> +                                   NTSC_HBP_DURATION_TYP_NS,
> +                                   NTSC_HBP_DURATION_MAX_NS),
> +                       PARAM_RANGE(NTSC_HBLK_DURATION_MIN_NS,
> +                                   NTSC_HBLK_DURATION_TYP_NS,
> +                                   NTSC_HBLK_DURATION_MAX_NS),
> +                       16,
> +                       PARAM_FIELD(3, 3),
> +                       PARAM_FIELD(3, 3),
> +                       PARAM_FIELD(16, 17)),
> +     TV_MODE_PARAMETER(DRM_MODE_ANALOG_PAL,
> +                       PAL_LINES_NUMBER,
> +                       PAL_LINE_DURATION_NS,
> +                       PARAM_RANGE(PAL_HACT_DURATION_MIN_NS,
> +                                   PAL_HACT_DURATION_TYP_NS,
> +                                   PAL_HACT_DURATION_MAX_NS),
> +                       PARAM_RANGE(PAL_HFP_DURATION_MIN_NS,
> +                                   PAL_HFP_DURATION_TYP_NS,
> +                                   PAL_HFP_DURATION_MAX_NS),
> +                       PARAM_RANGE(PAL_HSLEN_DURATION_MIN_NS,
> +                                   PAL_HSLEN_DURATION_TYP_NS,
> +                                   PAL_HSLEN_DURATION_MAX_NS),
> +                       PARAM_RANGE(PAL_HBP_DURATION_MIN_NS,
> +                                   PAL_HBP_DURATION_TYP_NS,
> +                                   PAL_HBP_DURATION_MAX_NS),
> +                       PARAM_RANGE(PAL_HBLK_DURATION_MIN_NS,
> +                                   PAL_HBLK_DURATION_TYP_NS,
> +                                   PAL_HBLK_DURATION_MAX_NS),
> +                       12,
> +
> +                       /*
> +                        * The front porch is actually 6 short sync
> +                        * pulses for the even field, and 5 for the
> +                        * odd field. Each sync takes half a life so
> +                        * the odd field front porch is shorter by
> +                        * half a line.
> +                        *
> +                        * In progressive, we're supposed to use 6
> +                        * pulses, so we're fine there
> +                        */
> +                       PARAM_FIELD(3, 2),
> +
> +                       /*
> +                        * The vsync length is 5 long sync pulses,
> +                        * each field taking half a line. We're
> +                        * shorter for both fields by half a line.
> +                        *
> +                        * In progressive, we're supposed to use 5
> +                        * pulses, so we're off by half
> +                        * a line.
> +                        *
> +                        * In interlace, we're now off by half a line
> +                        * for the even field and one line for the odd
> +                        * field.
> +                        */
> +                       PARAM_FIELD(3, 3),
> +
> +                       /*
> +                        * The back porch starts with post-equalizing
> +                        * pulses, consisting in 5 short sync pulses
> +                        * for the even field, 4 for the odd field. In
> +                        * progressive, it's 5 short syncs.
> +                        *
> +                        * In progressive, we thus have 2.5 lines,
> +                        * plus the 0.5 line we were missing
> +                        * previously, so we should use 3 lines.
> +                        *
> +                        * In interlace, the even field is in the
> +                        * exact same case than progressive. For the
> +                        * odd field, we should be using 2 lines but
> +                        * we're one line short, so we'll make up for
> +                        * it here by using 3.
> +                        *
> +                        * The entire blanking area is supposed to
> +                        * take 25 lines, so we also need to account
> +                        * for the rest of the blanking area that
> +                        * can't be in either the front porch or sync
> +                        * period.
> +                        */
> +                       PARAM_FIELD(19, 20)),
> +};

Nit: setting vbp limits like that makes it impossible to use
drm_analog_tv_mode() to generate modes that include the VBI for e.g. emitting
teletext.

This probably doesn't matter, as it can still be created as a custom mode from
userspace, hence I'm mentioning it as a nit.

> +              * By convention, NSTC (aka 525/60) systems start with

Typo: s/NSTC/NTSC/

Best regards,
Mateusz Kwiatkowski

Reply via email to