On 14/12/17 00:33, Tim Harvey wrote: > On Tue, Dec 12, 2017 at 4:18 AM, Hans Verkuil <hverkuil@xxxxxxxxx> 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: >>> 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. >> >> 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. The video-interfaces.txt bindings text defines bus-width and data-shift. The last one would define 'which 8'. Only if the hardware can do weird stuff like using pins 4-7 and 16-19 would this be insufficient. But in that case we would need something more detailed in video-interfaces.txt. Frankly, unless you've seen anyone do something weird like that I would stick to bus-width and data-shift. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
