linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
@ 2018-12-10 16:00 Thierry Reding
  2018-12-10 16:00 ` [PATCH 2/2] media: tegra-cec: Export OF device ID match table Thierry Reding
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Thierry Reding @ 2018-12-10 16:00 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab
  Cc: Jonathan Hunter, linux-media, linux-tegra

From: Thierry Reding <treding@nvidia.com>

The CEC controller found on Tegra186 and Tegra194 is the same as on
earlier generations.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/media/platform/tegra-cec/tegra_cec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c b/drivers/media/platform/tegra-cec/tegra_cec.c
index aba488cd0e64..8a1e10d008d0 100644
--- a/drivers/media/platform/tegra-cec/tegra_cec.c
+++ b/drivers/media/platform/tegra-cec/tegra_cec.c
@@ -472,6 +472,8 @@ static const struct of_device_id tegra_cec_of_match[] = {
 	{ .compatible = "nvidia,tegra114-cec", },
 	{ .compatible = "nvidia,tegra124-cec", },
 	{ .compatible = "nvidia,tegra210-cec", },
+	{ .compatible = "nvidia,tegra186-cec", },
+	{ .compatible = "nvidia,tegra194-cec", },
 	{},
 };
 
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] media: tegra-cec: Export OF device ID match table
  2018-12-10 16:00 [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Thierry Reding
@ 2018-12-10 16:00 ` Thierry Reding
  2018-12-10 17:07 ` [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Hans Verkuil
  2018-12-11  9:26 ` Hans Verkuil
  2 siblings, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2018-12-10 16:00 UTC (permalink / raw)
  To: Hans Verkuil, Mauro Carvalho Chehab
  Cc: Jonathan Hunter, linux-media, linux-tegra

From: Thierry Reding <treding@nvidia.com>

Exporting the OF device ID match table allows udev to automatically load
the module upon receiving an "ADD" uevent for the CEC controller device.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/media/platform/tegra-cec/tegra_cec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c b/drivers/media/platform/tegra-cec/tegra_cec.c
index 8a1e10d008d0..4e9f91528f77 100644
--- a/drivers/media/platform/tegra-cec/tegra_cec.c
+++ b/drivers/media/platform/tegra-cec/tegra_cec.c
@@ -476,6 +476,7 @@ static const struct of_device_id tegra_cec_of_match[] = {
 	{ .compatible = "nvidia,tegra194-cec", },
 	{},
 };
+MODULE_DEVICE_TABLE(of, tegra_cec_of_match);
 
 static struct platform_driver tegra_cec_driver = {
 	.driver = {
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-10 16:00 [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Thierry Reding
  2018-12-10 16:00 ` [PATCH 2/2] media: tegra-cec: Export OF device ID match table Thierry Reding
@ 2018-12-10 17:07 ` Hans Verkuil
  2018-12-10 20:59   ` Thierry Reding
  2018-12-11  9:26 ` Hans Verkuil
  2 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2018-12-10 17:07 UTC (permalink / raw)
  To: Thierry Reding, Hans Verkuil, Mauro Carvalho Chehab
  Cc: Jonathan Hunter, linux-media, linux-tegra

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.

HDMI inputs CAN share the CEC line, but never outputs. There should have been two
CEC blocks, one for each HDMI output.

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.

So before I can merge this I need to know first how you plan to handle this HW bug.

Regards,

	Hans

> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/media/platform/tegra-cec/tegra_cec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c b/drivers/media/platform/tegra-cec/tegra_cec.c
> index aba488cd0e64..8a1e10d008d0 100644
> --- a/drivers/media/platform/tegra-cec/tegra_cec.c
> +++ b/drivers/media/platform/tegra-cec/tegra_cec.c
> @@ -472,6 +472,8 @@ static const struct of_device_id tegra_cec_of_match[] = {
>  	{ .compatible = "nvidia,tegra114-cec", },
>  	{ .compatible = "nvidia,tegra124-cec", },
>  	{ .compatible = "nvidia,tegra210-cec", },
> +	{ .compatible = "nvidia,tegra186-cec", },
> +	{ .compatible = "nvidia,tegra194-cec", },
>  	{},
>  };
>  
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-10 17:07 ` [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Hans Verkuil
@ 2018-12-10 20:59   ` Thierry Reding
  2018-12-11  9:19     ` Hans Verkuil
  0 siblings, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2018-12-10 20:59 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-10 20:59   ` Thierry Reding
@ 2018-12-11  9:19     ` Hans Verkuil
  2018-12-11  9:38       ` Thierry Reding
  0 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2018-12-11  9:19 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-10 16:00 [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Thierry Reding
  2018-12-10 16:00 ` [PATCH 2/2] media: tegra-cec: Export OF device ID match table Thierry Reding
  2018-12-10 17:07 ` [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Hans Verkuil
@ 2018-12-11  9:26 ` Hans Verkuil
  2018-12-11  9:39   ` Thierry Reding
  2 siblings, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2018-12-11  9:26 UTC (permalink / raw)
  To: Thierry Reding, Hans Verkuil, Mauro Carvalho Chehab
  Cc: Jonathan Hunter, linux-media, linux-tegra

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.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/media/platform/tegra-cec/tegra_cec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c b/drivers/media/platform/tegra-cec/tegra_cec.c
> index aba488cd0e64..8a1e10d008d0 100644
> --- a/drivers/media/platform/tegra-cec/tegra_cec.c
> +++ b/drivers/media/platform/tegra-cec/tegra_cec.c
> @@ -472,6 +472,8 @@ static const struct of_device_id tegra_cec_of_match[] = {
>  	{ .compatible = "nvidia,tegra114-cec", },
>  	{ .compatible = "nvidia,tegra124-cec", },
>  	{ .compatible = "nvidia,tegra210-cec", },
> +	{ .compatible = "nvidia,tegra186-cec", },
> +	{ .compatible = "nvidia,tegra194-cec", },
>  	{},
>  };
>  
> 

Applying: media: tegra-cec: Support Tegra186 and Tegra194
WARNING: DT compatible string "nvidia,tegra186-cec" appears un-documented -- check ./Documentation/devicetree/bindings/
#9: FILE: drivers/media/platform/tegra-cec/tegra_cec.c:475:
+       { .compatible = "nvidia,tegra186-cec", },

WARNING: DT compatible string "nvidia,tegra194-cec" appears un-documented -- check ./Documentation/devicetree/bindings/
#10: FILE: drivers/media/platform/tegra-cec/tegra_cec.c:476:
+       { .compatible = "nvidia,tegra194-cec", },

I need an additional patch adding the new bindings to
Documentation/devicetree/bindings/media/tegra-cec.txt.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-11  9:19     ` Hans Verkuil
@ 2018-12-11  9:38       ` Thierry Reding
  2018-12-11  9:39         ` Hans Verkuil
  2018-12-11  9:40         ` Hans Verkuil
  0 siblings, 2 replies; 11+ messages in thread
From: Thierry Reding @ 2018-12-11  9:38 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 3237 bytes --]

On Tue, Dec 11, 2018 at 10:19:48AM +0100, Hans Verkuil wrote:
> 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).

I wasn't aware that anyone was using a Tegra with support for multiple
HDMI outputs. Do you have a contact that you can forward this kind of
request to? It certainly sounds like something that would be useful to
add in future chips if there's a customer need.

I can also forward this internally, but I expect it to have more weight
coming directly from Cisco. =)

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

No worries.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-11  9:38       ` Thierry Reding
@ 2018-12-11  9:39         ` Hans Verkuil
  2018-12-11  9:40         ` Hans Verkuil
  1 sibling, 0 replies; 11+ messages in thread
From: Hans Verkuil @ 2018-12-11  9:39 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

On 12/11/18 10:38 AM, Thierry Reding wrote:
> On Tue, Dec 11, 2018 at 10:19:48AM +0100, Hans Verkuil wrote:
>> 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).
> 
> I wasn't aware that anyone was using a Tegra with support for multiple
> HDMI outputs. Do you have a contact that you can forward this kind of
> request to? It certainly sounds like something that would be useful to
> add in future chips if there's a customer need.
> 
> I can also forward this internally, but I expect it to have more weight
> coming directly from Cisco. =)
> 
>> Apologies for the confusion, I should never send emails after 5pm :-)
> 
> No worries.
> 
> Thierry
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-11  9:26 ` Hans Verkuil
@ 2018-12-11  9:39   ` Thierry Reding
  0 siblings, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2018-12-11  9:39 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 1968 bytes --]

On Tue, Dec 11, 2018 at 10:26:14AM +0100, Hans Verkuil wrote:
> 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.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/media/platform/tegra-cec/tegra_cec.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/media/platform/tegra-cec/tegra_cec.c b/drivers/media/platform/tegra-cec/tegra_cec.c
> > index aba488cd0e64..8a1e10d008d0 100644
> > --- a/drivers/media/platform/tegra-cec/tegra_cec.c
> > +++ b/drivers/media/platform/tegra-cec/tegra_cec.c
> > @@ -472,6 +472,8 @@ static const struct of_device_id tegra_cec_of_match[] = {
> >  	{ .compatible = "nvidia,tegra114-cec", },
> >  	{ .compatible = "nvidia,tegra124-cec", },
> >  	{ .compatible = "nvidia,tegra210-cec", },
> > +	{ .compatible = "nvidia,tegra186-cec", },
> > +	{ .compatible = "nvidia,tegra194-cec", },
> >  	{},
> >  };
> >  
> > 
> 
> Applying: media: tegra-cec: Support Tegra186 and Tegra194
> WARNING: DT compatible string "nvidia,tegra186-cec" appears un-documented -- check ./Documentation/devicetree/bindings/
> #9: FILE: drivers/media/platform/tegra-cec/tegra_cec.c:475:
> +       { .compatible = "nvidia,tegra186-cec", },
> 
> WARNING: DT compatible string "nvidia,tegra194-cec" appears un-documented -- check ./Documentation/devicetree/bindings/
> #10: FILE: drivers/media/platform/tegra-cec/tegra_cec.c:476:
> +       { .compatible = "nvidia,tegra194-cec", },

Ugh... I usually have a git hook in place to catch this kind of mistake,
but it's currently disabled because it was getting in the way of some
large rebases I've been doing lately...

> I need an additional patch adding the new bindings to
> Documentation/devicetree/bindings/media/tegra-cec.txt.

Will send a patch right away.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-11  9:38       ` Thierry Reding
  2018-12-11  9:39         ` Hans Verkuil
@ 2018-12-11  9:40         ` Hans Verkuil
  2018-12-11 10:00           ` Thierry Reding
  1 sibling, 1 reply; 11+ messages in thread
From: Hans Verkuil @ 2018-12-11  9:40 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

(Resend, this time with a proper reply)

On 12/11/18 10:38 AM, Thierry Reding wrote:
> On Tue, Dec 11, 2018 at 10:19:48AM +0100, Hans Verkuil wrote:
>> 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).
> 
> I wasn't aware that anyone was using a Tegra with support for multiple
> HDMI outputs. Do you have a contact that you can forward this kind of
> request to? It certainly sounds like something that would be useful to
> add in future chips if there's a customer need.

We have contacts, and we did report it, but nothing happened with it
AFAIK. It was likely too late for changes to the Tegra194 design in any
case.

Regards,

	Hans

> 
> I can also forward this internally, but I expect it to have more weight
> coming directly from Cisco. =)
> 
>> Apologies for the confusion, I should never send emails after 5pm :-)
> 
> No worries.
> 
> Thierry
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194
  2018-12-11  9:40         ` Hans Verkuil
@ 2018-12-11 10:00           ` Thierry Reding
  0 siblings, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2018-12-11 10:00 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, Mauro Carvalho Chehab, Jonathan Hunter,
	linux-media, linux-tegra

[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]

On Tue, Dec 11, 2018 at 10:40:55AM +0100, Hans Verkuil wrote:
> (Resend, this time with a proper reply)
> 
> On 12/11/18 10:38 AM, Thierry Reding wrote:
> > On Tue, Dec 11, 2018 at 10:19:48AM +0100, Hans Verkuil wrote:
> >> 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).
> > 
> > I wasn't aware that anyone was using a Tegra with support for multiple
> > HDMI outputs. Do you have a contact that you can forward this kind of
> > request to? It certainly sounds like something that would be useful to
> > add in future chips if there's a customer need.
> 
> We have contacts, and we did report it, but nothing happened with it
> AFAIK. It was likely too late for changes to the Tegra194 design in any
> case.

Okay. I'll follow up internally, see if I can find out what the story
is.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-12-11 10:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10 16:00 [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Thierry Reding
2018-12-10 16:00 ` [PATCH 2/2] media: tegra-cec: Export OF device ID match table Thierry Reding
2018-12-10 17:07 ` [PATCH 1/2] media: tegra-cec: Support Tegra186 and Tegra194 Hans Verkuil
2018-12-10 20:59   ` Thierry Reding
2018-12-11  9:19     ` Hans Verkuil
2018-12-11  9:38       ` Thierry Reding
2018-12-11  9:39         ` Hans Verkuil
2018-12-11  9:40         ` Hans Verkuil
2018-12-11 10:00           ` Thierry Reding
2018-12-11  9:26 ` Hans Verkuil
2018-12-11  9:39   ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).