linux-kernel - Re: [PATCH v10 3/6] media: platform: synopsys: Add support for HDMI input driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <D82H4F6J4V4V.2BLHVUVD3BVX9@cknow.org>
Date: Wed, 26 Feb 2025 16:14:02 +0100
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Dmitry Osipenko" <dmitry.osipenko@...labora.com>, "Shreeya Patel"
 <shreeya.patel@...labora.com>, "Heiko Stuebner" <heiko@...ech.de>, "Mauro
 Carvalho Chehab" <mchehab@...nel.org>, "Hans Verkuil" <hverkuil@...all.nl>,
 "Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski"
 <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
 <jose.abreu@...opsys.com>, <nelson.costa@...opsys.com>,
 <shawn.wen@...k-chips.com>, <nicolas.dufresne@...labora.com>, "Sebastian
 Reichel" <sebastian.reichel@...labora.com>
Cc: <kernel@...labora.com>, <linux-media@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <linux-rockchip@...ts.infradead.org>, "Tim Surber" <me@...surber.de>,
 "Christophe JAILLET" <christophe.jaillet@...adoo.fr>, "Diederik de Haas"
 <didi.debian@...ow.org>
Subject: Re: [PATCH v10 3/6] media: platform: synopsys: Add support for HDMI
 input driver

Hi,

On Tue Feb 25, 2025 at 7:30 PM CET, Dmitry Osipenko wrote:
> From: Shreeya Patel <shreeya.patel@...labora.com>
>
> Add initial support for the Synopsys DesignWare HDMI RX
> Controller Driver used by Rockchip RK3588. The driver
> supports:
>  - HDMI 1.4b and 2.0 modes (HDMI 4k@...z)
>  - RGB888, YUV422, YUV444 and YCC420 pixel formats
>  - CEC
>  - EDID configuration
>
> The hardware also has Audio and HDCP capabilities, but these are
> not yet supported by the driver.
>
> Co-developed-by: Dingxian Wen <shawn.wen@...k-chips.com>
> Signed-off-by: Dingxian Wen <shawn.wen@...k-chips.com>
> Signed-off-by: Shreeya Patel <shreeya.patel@...labora.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>
> ---
>  drivers/media/platform/Kconfig                |    1 +
>  drivers/media/platform/Makefile               |    1 +
>  drivers/media/platform/synopsys/Kconfig       |    3 +
>  drivers/media/platform/synopsys/Makefile      |    2 +
>  .../media/platform/synopsys/hdmirx/Kconfig    |   27 +
>  .../media/platform/synopsys/hdmirx/Makefile   |    4 +
>  .../platform/synopsys/hdmirx/snps_hdmirx.c    | 2756 +++++++++++++++++
>  .../platform/synopsys/hdmirx/snps_hdmirx.h    |  394 +++
>  .../synopsys/hdmirx/snps_hdmirx_cec.c         |  284 ++
>  .../synopsys/hdmirx/snps_hdmirx_cec.h         |   44 +
>  10 files changed, 3516 insertions(+)
>  create mode 100644 drivers/media/platform/synopsys/Kconfig
>  create mode 100644 drivers/media/platform/synopsys/Makefile
>  create mode 100644 drivers/media/platform/synopsys/hdmirx/Kconfig
>  create mode 100644 drivers/media/platform/synopsys/hdmirx/Makefile
>  create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
>  create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
>  create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx_cec.c
>  create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx_cec.h
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 85d2627776b6..9287faafdce5 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -85,6 +85,7 @@ source "drivers/media/platform/rockchip/Kconfig"
>  source "drivers/media/platform/samsung/Kconfig"
>  source "drivers/media/platform/st/Kconfig"
>  source "drivers/media/platform/sunxi/Kconfig"
> +source "drivers/media/platform/synopsys/Kconfig"
>  source "drivers/media/platform/ti/Kconfig"
>  source "drivers/media/platform/verisilicon/Kconfig"
>  source "drivers/media/platform/via/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index ace4e34483dd..6fd7db0541c7 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -28,6 +28,7 @@ obj-y += rockchip/
>  obj-y += samsung/
>  obj-y += st/
>  obj-y += sunxi/
> +obj-y += synopsys/
>  obj-y += ti/
>  obj-y += verisilicon/
>  obj-y += via/
> diff --git a/drivers/media/platform/synopsys/Kconfig b/drivers/media/platform/synopsys/Kconfig
> new file mode 100644
> index 000000000000..4fd521f78425
> --- /dev/null
> +++ b/drivers/media/platform/synopsys/Kconfig
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +source "drivers/media/platform/synopsys/hdmirx/Kconfig"
> diff --git a/drivers/media/platform/synopsys/Makefile b/drivers/media/platform/synopsys/Makefile
> new file mode 100644
> index 000000000000..3b12c574dd67
> --- /dev/null
> +++ b/drivers/media/platform/synopsys/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-y += hdmirx/
> diff --git a/drivers/media/platform/synopsys/hdmirx/Kconfig b/drivers/media/platform/synopsys/hdmirx/Kconfig
> new file mode 100644
> index 000000000000..3f96f6c97cf0
> --- /dev/null
> +++ b/drivers/media/platform/synopsys/hdmirx/Kconfig
> @@ -0,0 +1,27 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config VIDEO_SYNOPSYS_HDMIRX
> +	tristate "Synopsys DesignWare HDMI Receiver driver"
> +	depends on VIDEO_DEV
> +	select MEDIA_CONTROLLER
> +	select VIDEO_V4L2_SUBDEV_API
> +	select VIDEOBUF2_DMA_CONTIG
> +	select CEC_CORE
> +	select CEC_NOTIFIER
> +	select HDMI
> +	help
> +	  Support for Synopsys HDMI HDMI RX Controller.
> +	  This driver supports HDMI 2.0 version.
> +
> +	  To compile this driver as a module, choose M here. The module
> +	  will be called synopsys_hdmirx.
> +
> +config VIDEO_SYNOPSYS_HDMIRX_LOAD_DEFAULT_EDID
> +	bool "Load default EDID"
> +	depends on VIDEO_SYNOPSYS_HDMIRX
> +	help
> +	  Preload default EDID (Extended Display Identification Data).

I see some value in making explicit what you mean by EDID ...

> +	  EDID contains information about the capabilities of the display,
> +	  such as supported resolutions, refresh rates, and audio formats.

... I do not think a/this Kconfig needs to explain it.

> +
> +	  Enabling this option is recommended for a non-production use-cases.

My guess is that it could be useful for development/debugging/CI work
which possibly do not have an actual device (monitor) which provides
actual EDID data? Is CI work the reason why you (initially) enabled it
in the defconfig?
But when you have an actual monitor (=production use-case?), you
(really) do not want it? Would it be harmless if 'still' enabled?

Thus a more extensive description what *this* Kconfig item does and why
and when I want to enable it or not, seems more useful to me.

Cheers,
  Diederik


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)