Thread (23 messages) 23 messages, 5 authors, 2017-12-18

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

From: Tim Harvey <hidden>
Date: 2017-12-13 23:33:39
Also in: alsa-devel, linux-media, lkml

On Tue, Dec 12, 2017 at 4:18 AM, Hans Verkuil [off-list ref] wrote:
Hi Tim,

Sorry for the delay, I needed to find some time to think about this.

On 11/16/17 05:30, Rob Herring wrote:
quoted
On Wed, Nov 15, 2017 at 10:31:14AM -0800, Tim Harvey wrote:
quoted
On Wed, Nov 15, 2017 at 7:52 AM, Rob Herring [off-list ref] wrote:
quoted
On Thu, Nov 09, 2017 at 10:45:34AM -0800, Tim Harvey wrote:
quoted
Add support for the TDA1997x HDMI receivers.

Cc: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
---
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:
quoted
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.
quoted
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.
I don't think it is right to store this in the DT. How you map the output pins
is a driver thing. So when you are requested to enumerate the mediabus formats
(include/uapi/linux/media-bus-format.h) you support, you do so based on the
capabilities of the hardware, and when a format is requested you program the
hardware accordingly.

The device tree should describe the physical characteristics like the number
of pins that are hooked up (i.e. are there 24, 30 or 36 pins connected).

These vidout-portcfg mappings do not appear to describe physical properties
but really register settings.
Hans,

They are register settings that define which bits on the internal data
bus are mapped to which pins on the package. Internally these parts
have a 36bit video data bus but externally the tda19971 only has 24
pins and even then perhaps only 8 are hooked up if using BT656 and the
registers also define 'which 8' as it could have been connected to the
upper or lower part of the bus. So while the bindings from
video-interfaces.txt provide bus-width and details of the sync
signals, additional hardware-specific interconnect details such as how
the video bits are shifted/mixed on the output bus are needed here.
This is why I feel vidout-portcfg should be a dt property vs something
like a module param.

Regards,

Tim
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help