Linux-Amlogic Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
@ 2021-04-16  9:27 Neil Armstrong
  2021-04-16  9:27 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Neil Armstrong
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Neil Armstrong @ 2021-04-16  9:27 UTC (permalink / raw)
  To: jonas, jernej.skrabec, robert.foss, Laurent.pinchart
  Cc: dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong

This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.

On some SoCs, the CEC functionality is enabled in the IP config bits, but the
CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
confusion when the DW-HDMI CEC controller can't communicate on the bus.

Jernej Skrabec (1):
  drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
    driver

Neil Armstrong (1):
  drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver

 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
 drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
 include/drm/bridge/dw_hdmi.h              | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver
  2021-04-16  9:27 [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver Neil Armstrong
@ 2021-04-16  9:27 ` Neil Armstrong
  2021-04-16  9:27 ` [PATCH 2/2] drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver Neil Armstrong
  2021-04-16  9:58 ` [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of " Laurent Pinchart
  2 siblings, 0 replies; 9+ messages in thread
From: Neil Armstrong @ 2021-04-16  9:27 UTC (permalink / raw)
  To: jonas, jernej.skrabec, robert.foss, Laurent.pinchart
  Cc: dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong

From: Jernej Skrabec <jernej.skrabec@siol.net>

This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.

On some SoCs, the CEC functionality is enabled in the IP config bits, but the
CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
confusion when the DW-HDMI CEC controller can't communicate on the bus.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
 include/drm/bridge/dw_hdmi.h              | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index dda4fa9a1a08..ae97513ef886 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3421,7 +3421,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 		hdmi->audio = platform_device_register_full(&pdevinfo);
 	}
 
-	if (config0 & HDMI_CONFIG0_CEC) {
+	if (!plat_data->disable_cec && (config0 & HDMI_CONFIG0_CEC)) {
 		cec.hdmi = hdmi;
 		cec.ops = &dw_hdmi_cec_ops;
 		cec.irq = irq;
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index ea34ca146b82..6a5716655619 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -153,6 +153,8 @@ struct dw_hdmi_plat_data {
 	const struct dw_hdmi_phy_config *phy_config;
 	int (*configure_phy)(struct dw_hdmi *hdmi, void *data,
 			     unsigned long mpixelclock);
+
+	unsigned int disable_cec : 1;
 };
 
 struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* [PATCH 2/2] drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
  2021-04-16  9:27 [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver Neil Armstrong
  2021-04-16  9:27 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Neil Armstrong
@ 2021-04-16  9:27 ` Neil Armstrong
  2021-04-16  9:58 ` [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of " Laurent Pinchart
  2 siblings, 0 replies; 9+ messages in thread
From: Neil Armstrong @ 2021-04-16  9:27 UTC (permalink / raw)
  To: jonas, jernej.skrabec, robert.foss, Laurent.pinchart
  Cc: dri-devel, linux-amlogic, linux-arm-kernel, linux-kernel, Neil Armstrong

On the Amlogic SoCs, the DW-HDMI HW support is here but the DW-HDMI CEC signal
is not connected to a physical pin, leading to confusion when the dw-hdmi cec
controller can't communicate on the bus.

Disable it to avoid exposing a non-functinal bus.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/meson_dw_hdmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index aad75a22dc33..2ed87cfdd735 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -1103,6 +1103,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	dw_plat_data->phy_data = meson_dw_hdmi;
 	dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
 	dw_plat_data->ycbcr_420_allowed = true;
+	dw_plat_data->disable_cec = true;
 
 	if (dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
 	    dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxm-dw-hdmi") ||
-- 
2.25.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
  2021-04-16  9:27 [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver Neil Armstrong
  2021-04-16  9:27 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Neil Armstrong
  2021-04-16  9:27 ` [PATCH 2/2] drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver Neil Armstrong
@ 2021-04-16  9:58 ` Laurent Pinchart
  2021-04-16 11:38   ` Neil Armstrong
  2 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2021-04-16  9:58 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: jonas, jernej.skrabec, robert.foss, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi Neil,

On Fri, Apr 16, 2021 at 11:27:35AM +0200, Neil Armstrong wrote:
> This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.
> 
> On some SoCs, the CEC functionality is enabled in the IP config bits, but the
> CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
> but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
> confusion when the DW-HDMI CEC controller can't communicate on the bus.

If we can't trust the CEC config bit, would it be better to not use it
at all, and instead let each platform glue logic tell whether to enable
CEC or not ?

> Jernej Skrabec (1):
>   drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
>     driver
> 
> Neil Armstrong (1):
>   drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
> 
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
>  include/drm/bridge/dw_hdmi.h              | 2 ++
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
  2021-04-16  9:58 ` [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of " Laurent Pinchart
@ 2021-04-16 11:38   ` Neil Armstrong
  2021-04-17  6:31     ` Jernej Škrabec
  2021-04-20 15:13     ` Hans Verkuil
  0 siblings, 2 replies; 9+ messages in thread
From: Neil Armstrong @ 2021-04-16 11:38 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: jonas, jernej.skrabec, robert.foss, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

On 16/04/2021 11:58, Laurent Pinchart wrote:
> Hi Neil,
> 
> On Fri, Apr 16, 2021 at 11:27:35AM +0200, Neil Armstrong wrote:
>> This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.
>>
>> On some SoCs, the CEC functionality is enabled in the IP config bits, but the
>> CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
>> but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
>> confusion when the DW-HDMI CEC controller can't communicate on the bus.
> 
> If we can't trust the CEC config bit, would it be better to not use it
> at all, and instead let each platform glue logic tell whether to enable
> CEC or not ?

Actually, the CEC config bit is right, the HW exists and should be functional, but
this bit doesn't tell if the CEC signal is connected to something.

This lies in the IP integration, like other bits under the "amlogic,meson-*-dw-hdmi"
umbrella.

The first attempt was by Hans using DT, but adding a property in DT for a vendor
specific compatible doesn't make sense. Another idea would be to describe the
CEC signal endpoint like we do for video signal, but I think this is out of scope and
this solution is much simpler and straightforward, and it's more an exception than
a general use case to solve.

Neil

> 
>> Jernej Skrabec (1):
>>   drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
>>     driver
>>
>> Neil Armstrong (1):
>>   drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
>>
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>  drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
>>  include/drm/bridge/dw_hdmi.h              | 2 ++
>>  3 files changed, 4 insertions(+), 1 deletion(-)
>>
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
  2021-04-16 11:38   ` Neil Armstrong
@ 2021-04-17  6:31     ` Jernej Škrabec
  2021-04-20 15:13     ` Hans Verkuil
  1 sibling, 0 replies; 9+ messages in thread
From: Jernej Škrabec @ 2021-04-17  6:31 UTC (permalink / raw)
  To: Laurent Pinchart, Neil Armstrong
  Cc: jonas, robert.foss, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, hverkuil-cisco

CC Hans Verkuil

Dne petek, 16. april 2021 ob 13:38:59 CEST je Neil Armstrong napisal(a):
> On 16/04/2021 11:58, Laurent Pinchart wrote:
> > Hi Neil,
> > 
> > On Fri, Apr 16, 2021 at 11:27:35AM +0200, Neil Armstrong wrote:
> >> This adds DW-HDMI driver a glue option to disable loading of the CEC
> >> sub-driver.
> >> 
> >> On some SoCs, the CEC functionality is enabled in the IP config bits, but
> >> the CEC bus is non-functional like on Amlogic SoCs, where the CEC config
> >> bit is set but the DW-HDMI CEC signal is not connected to a physical
> >> pin, leading to some confusion when the DW-HDMI CEC controller can't
> >> communicate on the bus.> 
> > If we can't trust the CEC config bit, would it be better to not use it
> > at all, and instead let each platform glue logic tell whether to enable
> > CEC or not ?
> 
> Actually, the CEC config bit is right, the HW exists and should be
> functional, but this bit doesn't tell if the CEC signal is connected to
> something.

I'm in favour of Neil's solution. Currently we have only one exception.

> 
> This lies in the IP integration, like other bits under the
> "amlogic,meson-*-dw-hdmi" umbrella.
> 
> The first attempt was by Hans using DT, but adding a property in DT for a
> vendor specific compatible doesn't make sense. Another idea would be to
> describe the CEC signal endpoint like we do for video signal, but I think
> this is out of scope and this solution is much simpler and straightforward,
> and it's more an exception than a general use case to solve.

Note that we still need DT property for disabling CEC. I have one Allwinner H3 
board where board designer decided to use GPIO CEC implementation instead of 
DW HDMI one (vendor Linux doesn't implement DW HDMI CEC driver). Other H3 
boards happily use DW HDMI CEC.

Best regards,
Jernej

> 
> Neil
> 
> >> Jernej Skrabec (1):
> >>   drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
> >>   
> >>     driver
> >> 
> >> Neil Armstrong (1):
> >>   drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
> >>  
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> >>  drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
> >>  include/drm/bridge/dw_hdmi.h              | 2 ++
> >>  3 files changed, 4 insertions(+), 1 deletion(-)





_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
  2021-04-16 11:38   ` Neil Armstrong
  2021-04-17  6:31     ` Jernej Škrabec
@ 2021-04-20 15:13     ` Hans Verkuil
  2021-04-20 15:19       ` Neil Armstrong
  1 sibling, 1 reply; 9+ messages in thread
From: Hans Verkuil @ 2021-04-20 15:13 UTC (permalink / raw)
  To: Neil Armstrong, Laurent Pinchart
  Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, robert.foss,
	linux-amlogic, linux-arm-kernel

On 16/04/2021 13:38, Neil Armstrong wrote:
> On 16/04/2021 11:58, Laurent Pinchart wrote:
>> Hi Neil,
>>
>> On Fri, Apr 16, 2021 at 11:27:35AM +0200, Neil Armstrong wrote:
>>> This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.
>>>
>>> On some SoCs, the CEC functionality is enabled in the IP config bits, but the
>>> CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
>>> but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
>>> confusion when the DW-HDMI CEC controller can't communicate on the bus.
>>
>> If we can't trust the CEC config bit, would it be better to not use it
>> at all, and instead let each platform glue logic tell whether to enable
>> CEC or not ?
> 
> Actually, the CEC config bit is right, the HW exists and should be functional, but
> this bit doesn't tell if the CEC signal is connected to something.
> 
> This lies in the IP integration, like other bits under the "amlogic,meson-*-dw-hdmi"
> umbrella.
> 
> The first attempt was by Hans using DT, but adding a property in DT for a vendor
> specific compatible doesn't make sense. Another idea would be to describe the
> CEC signal endpoint like we do for video signal, but I think this is out of scope and
> this solution is much simpler and straightforward, and it's more an exception than
> a general use case to solve.

While a DT property might not make sense in this particular case, I still
believe that it is a perfectly valid approach in general: whether or not
the CEC pin is connected is at the hardware level decision, it is not
something that software can detect. If the designer of the board didn't
connect it, then the only place you can define that is in the device tree.

Anyway, for meson I am fine with this solution. At least it prevents creating
a non-functioning cec device. So for this series:

Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Regards,

	Hans

> 
> Neil
> 
>>
>>> Jernej Skrabec (1):
>>>   drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
>>>     driver
>>>
>>> Neil Armstrong (1):
>>>   drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
>>>
>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>  drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
>>>  include/drm/bridge/dw_hdmi.h              | 2 ++
>>>  3 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
  2021-04-20 15:13     ` Hans Verkuil
@ 2021-04-20 15:19       ` Neil Armstrong
  2021-04-20 22:49         ` Laurent Pinchart
  0 siblings, 1 reply; 9+ messages in thread
From: Neil Armstrong @ 2021-04-20 15:19 UTC (permalink / raw)
  To: Hans Verkuil, Laurent Pinchart
  Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, robert.foss,
	linux-amlogic, linux-arm-kernel

On 20/04/2021 17:13, Hans Verkuil wrote:
> On 16/04/2021 13:38, Neil Armstrong wrote:
>> On 16/04/2021 11:58, Laurent Pinchart wrote:
>>> Hi Neil,
>>>
>>> On Fri, Apr 16, 2021 at 11:27:35AM +0200, Neil Armstrong wrote:
>>>> This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.
>>>>
>>>> On some SoCs, the CEC functionality is enabled in the IP config bits, but the
>>>> CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
>>>> but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
>>>> confusion when the DW-HDMI CEC controller can't communicate on the bus.
>>>
>>> If we can't trust the CEC config bit, would it be better to not use it
>>> at all, and instead let each platform glue logic tell whether to enable
>>> CEC or not ?
>>
>> Actually, the CEC config bit is right, the HW exists and should be functional, but
>> this bit doesn't tell if the CEC signal is connected to something.
>>
>> This lies in the IP integration, like other bits under the "amlogic,meson-*-dw-hdmi"
>> umbrella.
>>
>> The first attempt was by Hans using DT, but adding a property in DT for a vendor
>> specific compatible doesn't make sense. Another idea would be to describe the
>> CEC signal endpoint like we do for video signal, but I think this is out of scope and
>> this solution is much simpler and straightforward, and it's more an exception than
>> a general use case to solve.
> 
> While a DT property might not make sense in this particular case, I still
> believe that it is a perfectly valid approach in general: whether or not
> the CEC pin is connected is at the hardware level decision, it is not
> something that software can detect. If the designer of the board didn't
> connect it, then the only place you can define that is in the device tree.

Agreed, we need to define a smart way to declare CEC bus relationship in DT, the side
effect would be to handle this particular case.

> 
> Anyway, for meson I am fine with this solution. At least it prevents creating
> a non-functioning cec device. So for this series:
> 
> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Thanks,

Applying this serie to drm-misc-next

Neil

> 
> Regards,
> 
> 	Hans
> 
>>
>> Neil
>>
>>>
>>>> Jernej Skrabec (1):
>>>>   drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
>>>>     driver
>>>>
>>>> Neil Armstrong (1):
>>>>   drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
>>>>
>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>>  drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
>>>>  include/drm/bridge/dw_hdmi.h              | 2 ++
>>>>  3 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
  2021-04-20 15:19       ` Neil Armstrong
@ 2021-04-20 22:49         ` Laurent Pinchart
  0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2021-04-20 22:49 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Hans Verkuil, jernej.skrabec, jonas, linux-kernel, dri-devel,
	robert.foss, linux-amlogic, linux-arm-kernel

On Tue, Apr 20, 2021 at 05:19:52PM +0200, Neil Armstrong wrote:
> On 20/04/2021 17:13, Hans Verkuil wrote:
> > On 16/04/2021 13:38, Neil Armstrong wrote:
> >> On 16/04/2021 11:58, Laurent Pinchart wrote:
> >>> Hi Neil,
> >>>
> >>> On Fri, Apr 16, 2021 at 11:27:35AM +0200, Neil Armstrong wrote:
> >>>> This adds DW-HDMI driver a glue option to disable loading of the CEC sub-driver.
> >>>>
> >>>> On some SoCs, the CEC functionality is enabled in the IP config bits, but the
> >>>> CEC bus is non-functional like on Amlogic SoCs, where the CEC config bit is set
> >>>> but the DW-HDMI CEC signal is not connected to a physical pin, leading to some
> >>>> confusion when the DW-HDMI CEC controller can't communicate on the bus.
> >>>
> >>> If we can't trust the CEC config bit, would it be better to not use it
> >>> at all, and instead let each platform glue logic tell whether to enable
> >>> CEC or not ?
> >>
> >> Actually, the CEC config bit is right, the HW exists and should be functional, but
> >> this bit doesn't tell if the CEC signal is connected to something.
> >>
> >> This lies in the IP integration, like other bits under the "amlogic,meson-*-dw-hdmi"
> >> umbrella.
> >>
> >> The first attempt was by Hans using DT, but adding a property in DT for a vendor
> >> specific compatible doesn't make sense. Another idea would be to describe the
> >> CEC signal endpoint like we do for video signal, but I think this is out of scope and
> >> this solution is much simpler and straightforward, and it's more an exception than
> >> a general use case to solve.
> > 
> > While a DT property might not make sense in this particular case, I still
> > believe that it is a perfectly valid approach in general: whether or not
> > the CEC pin is connected is at the hardware level decision, it is not
> > something that software can detect. If the designer of the board didn't
> > connect it, then the only place you can define that is in the device tree.
> 
> Agreed, we need to define a smart way to declare CEC bus relationship in DT, the side
> effect would be to handle this particular case.

I wonder if it would make sense to use the OF graph bindings to describe
the connection between the CEC controller and the CEC "device" (which I
assume in most cases will be a DT node for a physical connector). Or is
this overkill ?

> > Anyway, for meson I am fine with this solution. At least it prevents creating
> > a non-functioning cec device. So for this series:
> > 
> > Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Thanks,
> 
> Applying this serie to drm-misc-next
> 
> >>>> Jernej Skrabec (1):
> >>>>   drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC
> >>>>     driver
> >>>>
> >>>> Neil Armstrong (1):
> >>>>   drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver
> >>>>
> >>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> >>>>  drivers/gpu/drm/meson/meson_dw_hdmi.c     | 1 +
> >>>>  include/drm/bridge/dw_hdmi.h              | 2 ++
> >>>>  3 files changed, 4 insertions(+), 1 deletion(-)

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16  9:27 [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver Neil Armstrong
2021-04-16  9:27 ` [PATCH 1/2] drm/bridge/synopsys: dw-hdmi: Add an option to suppress loading CEC driver Neil Armstrong
2021-04-16  9:27 ` [PATCH 2/2] drm/meson: dw-hdmi: disable DW-HDMI CEC sub-driver Neil Armstrong
2021-04-16  9:58 ` [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of " Laurent Pinchart
2021-04-16 11:38   ` Neil Armstrong
2021-04-17  6:31     ` Jernej Škrabec
2021-04-20 15:13     ` Hans Verkuil
2021-04-20 15:19       ` Neil Armstrong
2021-04-20 22:49         ` Laurent Pinchart

Linux-Amlogic Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-amlogic/0 linux-amlogic/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-amlogic linux-amlogic/ https://lore.kernel.org/linux-amlogic \
		linux-amlogic@lists.infradead.org
	public-inbox-index linux-amlogic

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-amlogic


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git