Messages in this thread | | | Subject | Re: [PATCH v6 2/3] media: i2c: adv748x: add adv748x driver | From | Hans Verkuil <> | Date | Mon, 3 Jul 2017 16:54:52 +0200 |
| |
On 07/03/2017 04:45 PM, Kieran Bingham wrote: >>> +#define ADV748X_HDMI_MAX_PIXELCLOCK 162000000 >> >> You probably based that on the 1600x1200p60 format? > > No idea I'm afraid - it's the value that was set when I recieved the driver... > >> >> 162MHz is a bit low for an adv receiver. The adv7604 and adv8742 have a max rate >> of 225 MHz. >> This should be documented in the datasheet. > > Hrm ... haven't found it yet - I'll keep digging....
This is often documented in the hardware datasheet, not the programming datasheet.
>>> +static void adv748x_hdmi_fill_format(struct adv748x_hdmi *hdmi, >>> + struct v4l2_mbus_framefmt *fmt) >>> +{ >>> + memset(fmt, 0, sizeof(*fmt)); >>> + >>> + fmt->code = MEDIA_BUS_FMT_RGB888_1X24; >>> + fmt->field = hdmi->timings.bt.interlaced ? >>> + V4L2_FIELD_INTERLACED : V4L2_FIELD_NONE; >> >> INTERLACED -> ALTERNATE. > > OK. > > OOI: Is this because of the removal of the interlaced cap - or because > V4L2_FIELD_INTERLACED is deprecated or such?
Unless the adv748x has a deinterlacer you will receive the fields as separate transfers.
FIELD_INTERLACED means that the two fields are combined into a single frame, but I doubt that that's what happens. FIELD_ALTERNATE means that the top and bottom fields are send one after the other.
FIELD_INTERLACED is very common for SDTV, but FIELD_ALTERNATE is almost always used for HDTV.
That reminds me: in adv748x_afe_fill_format you also set FIELD_INTERLACED, but is that correct? Shouldn't that also be FIELD_ALTERNATE?
> >>> + >>> + /* The colorspace depends on the AVI InfoFrame contents */
Missed that in the first review: add a 'TODO:' before this comment.
>>> + fmt->colorspace = V4L2_COLORSPACE_SRGB; >>> + >>> + fmt->width = hdmi->timings.bt.width; >>> + fmt->height = hdmi->timings.bt.height; >>> +}
Regards,
Hans
| |