From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
jernej.skrabec@siol.net, jonas@kwiboo.se,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
robert.foss@linaro.org, linux-amlogic@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 0/2] drm/bridge: dw-hdmi: disable loading of DW-HDMI CEC sub-driver
Date: Wed, 21 Apr 2021 01:49:36 +0300 [thread overview]
Message-ID: <YH9agHwLhEL27TA1@pendragon.ideasonboard.com> (raw)
In-Reply-To: <0821533b-9e6a-237e-3845-741e82de4ed9@baylibre.com>
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
prev parent reply other threads:[~2021-04-20 22:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YH9agHwLhEL27TA1@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=hverkuil@xs4all.nl \
--cc=jernej.skrabec@siol.net \
--cc=jonas@kwiboo.se \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=narmstrong@baylibre.com \
--cc=robert.foss@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).