Re: [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver

[Tim Harvey]

On Wed, Nov 15, 2017 at 8:30 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
> On Wed, Nov 15, 2017 at 10:31:14AM -0800, Tim Harvey wrote:
>> On Wed, Nov 15, 2017 at 7:52 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
>> > On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
>> >> Add support for the TDA1997x HDMI receivers.
>> >>
>> >> Cc: Hans Verkuil <hverkuil@xxxxxxxxx>
>> >> Signed-off-by: Tim Harvey <tharvey@xxxxxxxxxxxxx>
>> >> ---
>> >> v3:
>> >>  - use V4L2_DV_BT_FRAME_WIDTH/HEIGHT macros
>> >>  - fixed missing break
>> >>  - use only hdmi_infoframe_log for infoframe logging
>> >>  - simplify tda1997x_s_stream error handling
>> >>  - add delayed work proc to handle hotplug enable/disable
>> >>  - fix set_edid (disable HPD before writing, enable after)
>> >>  - remove enabling edid by default
>> >>  - initialize timings
>> >>  - take quant range into account in colorspace conversion
>> >>  - remove vendor/product tracking (we provide this in log_status via infoframes)
>> >>  - add v4l_controls
>> >>  - add more detail to log_status
>> >>  - calculate vhref generator timings
>> >>  - timing detection fixes (rounding errors, hswidth errors)
>> >>  - rename configure_input/configure_conv functions
>> >>
>> >> v2:
>> >>  - implement dv timings enum/cap
>> >>  - remove deprecated g_mbus_config op
>> >>  - fix dv_query_timings
>> >>  - add EDID get/set handling
>> >>  - remove max-pixel-rate support
>> >>  - add audio codec DAI support
>> >>  - change audio bindings
>> >> ---
>> >>  drivers/media/i2c/Kconfig            |    9 +
>> >>  drivers/media/i2c/Makefile           |    1 +
>> >>  drivers/media/i2c/tda1997x.c         | 3485 ++++++++++++++++++++++++++++++++++
>> >>  include/dt-bindings/media/tda1997x.h |   78 +
>> >
>> > This belongs with the binding documentation patch.
>> >
>>
>> Rob,
>>
>> Thanks - missed that. I will move it for v4.
>>
>> Regarding your previous comment to the v2 series:
>> > The rest of the binding looks fine, but I have some reservations about
>> > this. I think this should be common probably. There's been a few
>> > bindings for display recently that deal with the interface format. Maybe
>> > some vendor property is needed here to map a standard interface format
>> > back to pin configuration.
>>
>> I take it this is not an 'Ack' for the bindings?
>>
>> Which did you feel should be made common? I admit I was surprised
>> there wasn't a common binding for audio bus format (i2s|spdif) but if
>> you were referring to the video data that would probably be much more
>> complicated.
>
> The video data. Either you have to try to come up with some way to map
> color components to signals/pins (and even cycles) or you just enumerate
> the formats and keep adding to them when new ones appear. There's h/w
> that allows the former, but in the end you have to interoperate, so
> enumerating the formats is probably enough.
>
>> I was hoping one of the media/driver maintainers would respond to your
>> comment with thoughts as I'm not familiar with a very wide variety of
>> receivers.
>
> I am hoping, too.
>
> Rob

Hans,

Do you have any comment here regarding Rob's hope that there could be
some generic properties created for video port bindings? Anyone else
you know of who should chime in here?

The TDA1997x allows mapping its internal video output bus to its
physical pin in a fairly flexible way. I don't know how unique this is
to other chips.

Regards,

Tim