2017-11-15 18:47:01

by Rob Herring (Arm)

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

On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
> Add support for the TDA1997x HDMI receivers.
>
> Cc: Hans Verkuil <[email protected]>
> Signed-off-by: Tim Harvey <[email protected]>
> ---
> 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.

> include/media/i2c/tda1997x.h | 53 +
> 5 files changed, 3626 insertions(+)
> create mode 100644 drivers/media/i2c/tda1997x.c
> create mode 100644 include/dt-bindings/media/tda1997x.h
> create mode 100644 include/media/i2c/tda1997x.h

From 1584158064221344435@xxx Wed Nov 15 18:34:18 +0000 2017
X-GM-THRID: 1583615313022071316
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


2017-11-15 18:34:18

by Tim Harvey

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

On Wed, Nov 15, 2017 at 7:52 AM, Rob Herring <[email protected]> wrote:
> On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
>> Add support for the TDA1997x HDMI receivers.
>>
>> Cc: Hans Verkuil <[email protected]>
>> Signed-off-by: Tim Harvey <[email protected]>
>> ---
>> 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.

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.

Best regards,

Tim

From 1583615313022071316@xxx Thu Nov 09 18:47:31 +0000 2017
X-GM-THRID: 1583615313022071316
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread