Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

On 12/10/18 9:59 PM, Thierry Reding wrote:
> On Mon, Dec 10, 2018 at 06:07:10PM +0100, Hans Verkuil wrote:
>> Hi Thierry,
>>
>> On 12/10/18 5:00 PM, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> The CEC controller found on Tegra186 and Tegra194 is the same as on
>>> earlier generations.
>>
>> Well... at least for the Tegra186 there is a problem that needs to be addressed first.
>> No idea if this was solved for the Tegra194, it might be present there as well.
>>
>> The Tegra186 hardware connected the CEC lines of both HDMI outputs together. This is
>> a HW bug, and it means that only one of the two HDMI outputs can use the CEC block.
> 
> I don't know where you got that information from, but I can't find any
> indication of that in the documentation. My understanding is that there
> is a single CEC block that is completely independent and it is merely a
> decision of the board designer where to connect it. I'm not aware of any
> boards that expose more than a single CEC.

Sorry, my memory was not completely correct.

The problem is that the 186 can be configured with two HDMI outputs, but it has
only one CEC block. So CEC can be used for only one of the two. I checked the TRM
for the Tegra194 and that has up to four HDMI outputs, but still only one CEC
block.

And yes, it is the responsibility for the board designer to hook up the CEC pin
to only one of the outputs, but the TRM never explicitly mentions this and given
the general lack of knowledge about CEC it wouldn't surprise me at all if there
will be wrong board designs.

But be that as it may, the core problem remains: you cannot allow multiple
HDMI outputs to be connected to the same CEC device.

However, I now realize that your patches will actually work fine since each
HDMI connector tries to get a cec notifier for its own HDMI device, but the
tegra-cec driver will only register a notifier for the HDMI device pointed
to by the hdmi-phandle property. So only one of the HDMI devices will actually
get a working CEC.

Although if board designers mess this up and connect multiple CEC lines to
the same CEC pin, this would still break, but there is nothing that can be
done about that. I still believe the TRM should have made this clear since
it is not obvious. Even better would be to have the same number of CEC blocks
as there are configurable HDMI outputs. Typically, if you support CEC on one
HDMI output, you want to support it for all. And today that's not possible
without adding external CEC devices (as we - Cisco - do).

Apologies for the confusion, I should never send emails after 5pm :-)

Regards,

	Hans

> 
> You may already have access to the schematics, if not you can download
> them here:
> 
> 	https://developer.nvidia.com/embedded/dlc/jetson-tx1-tx2-developer-kit-carrier-board-c02-design-files
> 
> It's slightly annoying because it requires registration. But in those
> schematics you'll see that the HDMI_CEC pin is just routed directly from
> the processor module to the connector on the carrier board via the
> connector.
> 
>> HDMI inputs CAN share the CEC line, but never outputs. There should have been two
>> CEC blocks, one for each HDMI output.
> 
> Like I said, I don't think these are shared. The board design will have
> to choose which connector gets the SOR and CEC pins for HDMI. Typically
> the other SOR will be used for DisplayPort, not HDMI, though that would
> technically be possible. I think in case where there really were two
> HDMI connectors on a design, a decision would have to be made as to
> which one gets the CEC pin.
> 
>> It should not be possible to use the same CEC block for both HDMI
>> outputs on the 186. Ideally it should be a required dts property that
>> determines this. I'm not sure where that should happen. One option
>> might be to use the cec_notifier_get_conn() function so you can
>> register the CEC adapter for a specific connector only. For older
>> tegra versions the connector name would be NULL (i.e. don't care), for
>> the 186 (and perhaps 194) it would be a required property that tells
>> the CEC driver which connector it is associated with.
>>
>> Just a suggestion, there might be other ways to implement this as well.
> 
> Given the documentation that I have, I don't think we need to take any
> additional precautions. We just to hook up the CEC controller to the
> correct HDMI connector via the hdmi-phandle property.
> 
>> So before I can merge this I need to know first how you plan to handle
>> this HW bug.
> 
> I don't think this is an actual bug. It's more of a restriction of the
> SoC that allows only a single HDMI connector with CEC support.
> 
> Thierry
>