Skip to contentSkip to conversation

Synopsys Designware HDMI Video Capture Controller + PHY

PATCH Superseded Jose Abreu 4 participants 12 messages Jul 4, 2017 – Jul 7, 2017

Subsystems (2)

v6 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

Jose Abreu

author
#0 lore raw

v6 cover Synopsys Designware HDMI Video Capture Controller + PHY

The Synopsys Designware HDMI RX controller is an HDMI receiver controller that is responsible to process digital data that comes from a phy. The final result is a stream of raw video data that can then be connected to a video DMA, for example, and transfered into RAM so that it can be displayed.

The controller + phy available in this series natively support all HDMI 1.4 and HDMI 2.0 modes, including deep color. Although, the driver is quite in its initial stage and unfortunatelly only non deep color modes are supported. Also, audio is not yet supported in the driver (the controller has several audio output interfaces).

Version 6 addresses review comments from Hans Verkuil regarding CEC, HDCP 1.4 and adds the s_dv_timings() callback, as well as the V4L2_CID_DV_RX_POWER_PRESENT ctrl.

This series depends on the patch at [1].

This series was tested in a FPGA platform using an embedded platform called ARC AXS101.

Jose Abreu (4):
[media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver [media] platform: Add Synopsys Designware HDMI RX Controller Driver
MAINTAINERS: Add entry for Synopsys Designware HDMI drivers
dt-bindings: media: Document Synopsys Designware HDMI RX

Cc Carlos Palminha <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Rob Herring <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Sylwester Nawrocki <[email protected]>

[1] https://patchwork.linuxtv.org/patch/41834/

 .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  |   70 +
 MAINTAINERS                                        |    7 +
 drivers/media/platform/Kconfig                     |    2 +
 drivers/media/platform/Makefile                    |    2 +
 drivers/media/platform/dwc/Kconfig                 |   23 +
 drivers/media/platform/dwc/Makefile                |    2 +
 drivers/media/platform/dwc/dw-hdmi-phy-e405.c      |  844 +++++++++
 drivers/media/platform/dwc/dw-hdmi-phy-e405.h      |   63 +
 drivers/media/platform/dwc/dw-hdmi-rx.c            | 1809 ++++++++++++++++++++
 drivers/media/platform/dwc/dw-hdmi-rx.h            |  441 +++++
 include/media/dwc/dw-hdmi-phy-pdata.h              |  128 ++
 include/media/dwc/dw-hdmi-rx-pdata.h               |   70 +
 12 files changed, 3461 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
 create mode 100644 drivers/media/platform/dwc/Kconfig
 create mode 100644 drivers/media/platform/dwc/Makefile
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h
 create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h
 create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v6 1/4 platform: Add Synopsys Designware HDMI RX PHY e405 Driver

This adds support for the Synopsys Designware HDMI RX PHY e405. This phy receives and decodes HDMI video that is delivered to a controller.

Main features included in this driver are:
- Equalizer algorithm that chooses the phy best settings according to the detected HDMI cable characteristics.
- Support for scrambling
- Support for HDMI 2.0 modes up to 6G (HDMI 4k@60Hz).

The driver was implemented as a standalone V4L2 subdevice and the phy interface with the controller was implemented using V4L2 ioctls. I do not know if this is the best option but it is not possible to use the existing API functions directly as we need specific functions that will be called by the controller at specific configuration stages.

There is also a bidirectional communication between controller and phy:
The phy must provide functions that the controller will call (i.e. configuration functions) and the controller must provide read/write callbacks, as well as other specific functions.

Cc Carlos Palminha <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Sylwester Nawrocki <[email protected]>

Changes from v4:
- Use usleep_range (Sylwester)
- Remove some comments (Sylwester)
- Parse phy version from of_device_id (Sylwester)
- Use "cfg" instead of "cfg-clk" (Sylwester, Rob)
- Change some messages to dev_dbg (Sylwester)
Changes from v3:
- Use v4l2 async API (Sylwester)
- Use clock API (Sylwester)
- Add compatible string (Sylwester)
Changes from RFC:
- Remove a bunch of functions that can be collapsed into a single config() function
- Add comments for the callbacks and structures (Hans)

---
 drivers/media/platform/Kconfig                |   2 +
 drivers/media/platform/Makefile               |   2 +
 drivers/media/platform/dwc/Kconfig            |   8 +
 drivers/media/platform/dwc/Makefile           |   1 +
 drivers/media/platform/dwc/dw-hdmi-phy-e405.c | 844 ++++++++++++++++++++++++++
 drivers/media/platform/dwc/dw-hdmi-phy-e405.h |  63 ++
 include/media/dwc/dw-hdmi-phy-pdata.h         | 128 ++++
 7 files changed, 1048 insertions(+)
 create mode 100644 drivers/media/platform/dwc/Kconfig
 create mode 100644 drivers/media/platform/dwc/Makefile
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h
 create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h
drivers/media/platform/Kconfig +2 -0
index 1313cd5..47d4a50 100644
@@ -33,6 +33,8 @@ source "drivers/media/platform/omap/Kconfig"
 
 source "drivers/media/platform/blackfin/Kconfig"
 
+source "drivers/media/platform/dwc/Kconfig"
+
 config VIDEO_SH_VOU
 	tristate "SuperH VOU video output driver"
 	depends on MEDIA_CAMERA_SUPPORT
drivers/media/platform/Makefile +2 -0
index 9beadc7..e6a55fb 100644
@@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP)	+= mtk-mdp/
 obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)	+= mtk-jpeg/
 
 obj-$(CONFIG_VIDEO_QCOM_VENUS)		+= qcom/venus/
+
+obj-y	+= dwc/
drivers/media/platform/dwc/Kconfig new file +8 -0
new file mode 100644
index 0000000..361d38d
@@ -0,0 +1,8 @@
+config VIDEO_DWC_HDMI_PHY_E405
+	tristate "Synopsys Designware HDMI RX PHY e405 driver"
+	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	help
+	  Support for Synopsys Designware HDMI RX PHY. Version is e405.
+
+	  To compile this driver as a module, choose M here. The module
+	  will be called dw-hdmi-phy-e405.
drivers/media/platform/dwc/Makefile new file +1 -0
new file mode 100644
index 0000000..fc3b62c
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +844 -0

844 lines not on this page · raw message

drivers/media/platform/dwc/dw-hdmi-phy-e405.h new file +63 -0

63 lines not on this page · raw message

include/media/dwc/dw-hdmi-phy-pdata.h new file +128 -0

128 lines not on this page · raw message

Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v6 2/4 platform: Add Synopsys Designware HDMI RX Controller Driver

In reply to Jose Abreu

This is an initial submission for the Synopsys Designware HDMI RX Controller Driver. This driver interacts with a phy driver so that a communication between them is created and a video pipeline is configured.

The controller + phy pipeline can then be integrated into a fully featured system that can be able to receive video up to 4k@60Hz with deep color 48bit RGB, depending on the platform. Although, this initial version does not yet handle deep color modes.

This driver was implemented as a standard V4L2 subdevice and its main features are:
- Internal state machine that reconfigures phy until the video is not stable
- JTAG communication with phy
- Inter-module communication with phy driver
- Debug write/read ioctls

Some notes:
- RX sense controller (cable connection/disconnection) must be handled by the platform wrapper as this is not integrated into the controller RTL
- The same goes for EDID ROM's
- ZCAL calibration is needed only in FPGA platforms, in ASIC this is not needed
- The state machine is not an ideal solution as it creates a kthread but it is needed because some sources might not be very stable at sending the video (i.e. we must react accordingly).

Cc Carlos Palminha <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Sylwester Nawrocki <[email protected]>

Changes from v5:
- Removed HDCP 1.4 support (Hans)
- Removed some CEC debug messages (Hans)
- Add s_dv_timings callback (Hans)
- Add V4L2_CID_DV_RX_POWER_PRESENT ctrl (Hans)
Changes from v4:
- Add flag V4L2_SUBDEV_FL_HAS_DEVNODE (Sylwester)
- Remove some comments and change some messages to dev_dbg (Sylwester)
- Use v4l2_async_subnotifier_register() (Sylwester)
Changes from v3:
- Use v4l2 async API (Sylwester)
- Do not block waiting for phy
- Do not use busy waiting delays (Sylwester)
- Simplify dw_hdmi_power_on (Sylwester)
- Use clock API (Sylwester)
- Use compatible string (Sylwester)
- Minor fixes (Sylwester)
Changes from v2:
- Address review comments from Hans regarding CEC
- Use CEC notifier
- Enable SCDC
Changes from v1:
- Add support for CEC
- Correct typo errors
- Correctly detect interlaced video modes
- Correct VIC parsing
Changes from RFC:
- Add support for HDCP 1.4
- Fixup HDMI_VIC not being parsed (Hans)
- Send source change signal when powering off (Hans)
- Add a "wait stable delay"
- Detect interlaced video modes (Hans)
- Restrain g/s_register from reading/writing to HDCP regs (Hans)

---
 drivers/media/platform/dwc/Kconfig      |   15 +
 drivers/media/platform/dwc/Makefile     |    1 +
 drivers/media/platform/dwc/dw-hdmi-rx.c | 1809 +++++++++++++++++++++++++++++++
 drivers/media/platform/dwc/dw-hdmi-rx.h |  441 ++++++++
 include/media/dwc/dw-hdmi-rx-pdata.h    |   70 ++
 5 files changed, 2336 insertions(+)
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h
 create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h
drivers/media/platform/dwc/Kconfig +15 -0
index 361d38d..3ddccde 100644
@@ -6,3 +6,18 @@ config VIDEO_DWC_HDMI_PHY_E405
 
 	  To compile this driver as a module, choose M here. The module
 	  will be called dw-hdmi-phy-e405.
+
+config VIDEO_DWC_HDMI_RX
+	tristate "Synopsys Designware HDMI Receiver driver"
+	depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+	help
+	  Support for Synopsys Designware HDMI RX controller.
+
+	  To compile this driver as a module, choose M here. The module
+	  will be called dw-hdmi-rx.
+
+config VIDEO_DWC_HDMI_RX_CEC
+	bool
+	depends on VIDEO_DWC_HDMI_RX
+	select CEC_CORE
+	select CEC_NOTIFIER
drivers/media/platform/dwc/Makefile +1 -0
index fc3b62c..cd04ca9 100644
@@ -1 +1,2 @@
 obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o
+obj-$(CONFIG_VIDEO_DWC_HDMI_RX) += dw-hdmi-rx.o
drivers/media/platform/dwc/dw-hdmi-rx.c new file +1809 -0

1,809 lines not on this page · raw message

drivers/media/platform/dwc/dw-hdmi-rx.h new file +441 -0

441 lines not on this page · raw message

include/media/dwc/dw-hdmi-rx-pdata.h new file +70 -0

70 lines not on this page · raw message

Signature · 1 line
1.9.1
#3 lore raw

Re: [PATCH v6 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver

reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross…

Hi Jose,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20170706]
[cannot apply to v4.12]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jose-Abreu/Synopsys-Designware-HDMI-Video-Capture-Controller-PHY/20170707-041312
base:   git://linuxtv.org/media_tree.git master
config: blackfin-allyesconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0

reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin

All errors (new ones prefixed by >>):

drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_registered':

>> drivers/media//platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration of function 'v4l2_async_subnotifier_register' [-Werror=implicit-function-declaration]
     return v4l2_async_subnotifier_register(&dw_dev->sd,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_unregistered':
>> drivers/media//platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration of function 'v4l2_async_subnotifier_unregister' [-Werror=implicit-function-declaration]
     v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_parse_dt':
   drivers/media//platform/dwc/dw-hdmi-rx.c:1555:22: warning: unused variable 'notifier' [-Wunused-variable]
     struct device_node *notifier, *phy_node, *np = dw_dev->of_node;
                         ^~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_rx_probe':
   drivers/media//platform/dwc/dw-hdmi-rx.c:1765:1: warning: label 'err_phy' defined but not used [-Wunused-label]
    err_phy:
    ^~~~~~~

cc1: some warnings being treated as errors

vim +/v4l2_async_subnotifier_register +1452 drivers/media//platform/dwc/dw-hdmi-rx.c

1446 return ret;
1447 }
1448
1449 cec_register_cec_notifier(dw_dev->cec_adap, dw_dev->cec_notifier);
1450 dw_dev->registered = true;
1451

> 1452		return v4l2_async_subnotifier_register(&dw_dev->sd,
  1453				&dw_dev->v4l2_notifier);
  1454	}
  1455	
  1456	static void dw_hdmi_unregistered(struct v4l2_subdev *sd)
  1457	{
  1458		struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
  1459	
  1460		cec_unregister_adapter(dw_dev->cec_adap);
  1461		cec_notifier_put(dw_dev->cec_notifier);
> 1462		v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);

1463 }
1464
1465 static const struct v4l2_subdev_core_ops dw_hdmi_sd_core_ops = {

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author
#4 lore raw

Re: [PATCH v6 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver

Quoted 7 lines · kbuild test robot

drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_registered':

Quoted 1 line
Quoted 1 line

drivers/media//platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration of function 'v4l2_async_subnotifier_register' [-Werror=implicit-function-declaration]

     return v4l2_async_subnotifier_register(&dw_dev->sd,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_unregistered':

Quoted 1 line
Quoted 1 line

drivers/media//platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration of function 'v4l2_async_subnotifier_unregister' [-Werror=implicit-function-declaration]

     v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);

This one is expected. Required patch was not merged yet.

Quoted 9 lines
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_parse_dt':
   drivers/media//platform/dwc/dw-hdmi-rx.c:1555:22: warning: unused variable 'notifier' [-Wunused-variable]
     struct device_node *notifier, *phy_node, *np = dw_dev->of_node;
                         ^~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_rx_probe':
   drivers/media//platform/dwc/dw-hdmi-rx.c:1765:1: warning: label 'err_phy' defined but not used [-Wunused-label]
    err_phy:
    ^~~~~~~
Quoted 3 lines

cc1: some warnings being treated as errors

These two is due to cec not being enabled. I hate to wrap this around a #ifdef but is either this or extract cec to a separate file, which I don't think is the best solution because the cec code is quite small.

Best regards,
Jose Miguel Abreu

#5 lore raw

Re: [PATCH v6 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver

Quoted 24 lines · Jose Abreu
Quoted 7 lines · kbuild test robot

drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_registered':

Quoted 1 line
Quoted 1 line

drivers/media//platform/dwc/dw-hdmi-rx.c:1452:9: error: implicit declaration of function 'v4l2_async_subnotifier_register' [-Werror=implicit-function-declaration]

     return v4l2_async_subnotifier_register(&dw_dev->sd,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_unregistered':

Quoted 1 line
Quoted 1 line

drivers/media//platform/dwc/dw-hdmi-rx.c:1462:2: error: implicit declaration of function 'v4l2_async_subnotifier_unregister' [-Werror=implicit-function-declaration]

     v4l2_async_subnotifier_unregister(&dw_dev->v4l2_notifier);

This one is expected. Required patch was not merged yet.

Quoted 9 lines
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_parse_dt':
   drivers/media//platform/dwc/dw-hdmi-rx.c:1555:22: warning: unused variable 'notifier' [-Wunused-variable]
     struct device_node *notifier, *phy_node, *np = dw_dev->of_node;
                         ^~~~~~~~
   drivers/media//platform/dwc/dw-hdmi-rx.c: In function 'dw_hdmi_rx_probe':
   drivers/media//platform/dwc/dw-hdmi-rx.c:1765:1: warning: label 'err_phy' defined but not used [-Wunused-label]
    err_phy:
    ^~~~~~~
Quoted 3 lines

cc1: some warnings being treated as errors

Quoted 4 lines

These two is due to cec not being enabled. I hate to wrap this around a #ifdef but is either this or extract cec to a separate file, which I don't think is the best solution because the cec code is quite small.

The notifier functions can also be used if CEC_NOTIFIER isn't set. See cec-notifier.h for the stub functions.

That will help somewhat.

Regards,

Hans

author
#6 lore raw

PATCH v6 3/4 MAINTAINERS: Add entry for Synopsys Designware HDMI drivers

In reply to Jose Abreu

Add a entry for Synopsys Designware HDMI Receivers drivers and phys.

Cc Carlos Palminha <[email protected]>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)
MAINTAINERS +7 -0
index c4be6d4..7ebc6dd 100644
@@ -11354,6 +11354,13 @@ L:	[email protected]
 S:	Supported
 F:	drivers/net/ethernet/synopsys/
 
+SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS
+M:	Jose Abreu <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/media/platform/dwc/*
+F:	include/media/dwc/*
+
 SYNOPSYS DESIGNWARE I2C DRIVER
 M:	Jarkko Nikula <[email protected]>
 R:	Andy Shevchenko <[email protected]>
Signature · 1 line
1.9.1
author
#7 lore raw

PATCH v6 4/4 dt-bindings: media: Document Synopsys Designware HDMI RX

In reply to Jose Abreu

Document the bindings for the Synopsys Designware HDMI RX.

Cc Carlos Palminha <palminha-HKixBCOQz3hWk0Htik3J/[email protected]>
Cc Rob Herring <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Hans Verkuil <[email protected]>
Cc Sylwester Nawrocki <[email protected]>

Changes from v4:
- Use "cfg" instead of "cfg-clk" (Rob)
- Change node names (Rob)
Changes from v3:
- Document the new DT bindings suggested by Sylwester
Changes from v2:
- Document edid-phandle property

---
 .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +70 -0
new file mode 100644
index 0000000..449b8a2
@@ -0,0 +1,70 @@
+Synopsys DesignWare HDMI RX Decoder
+===================================
+
+This document defines device tree properties for the Synopsys DesignWare HDMI
+RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding
+specification by itself but is meant to be referenced by platform-specific
+device tree bindings.
+
+When referenced from platform device tree bindings the properties defined in
+this document are defined as follows.
+
+- compatible: Shall be "snps,dw-hdmi-rx".
+
+- reg: Memory mapped base address and length of the DWC HDMI RX registers.
+
+- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt.
+
+- clocks: Phandle to the config clock block.
+
+- clock-names: Shall be "cfg".
+
+- edid-phandle: phandle to the EDID handler block.
+
+- #address-cells: Shall be 1.
+
+- #size-cells: Shall be 0.
+
+You also have to create a subnode for phy driver. Phy properties are as follows.
+
+- compatible: Shall be "snps,dw-hdmi-phy-e405".
+
+- reg: Shall be JTAG address of phy.
+
+- clocks: Phandle for cfg clock.
+
+- clock-names:Shall be "cfg".
+
+A sample binding is now provided. The compatible string is for a SoC which has
+has a Synopsys DesignWare HDMI RX decoder inside.
+
+Example:
+
+dw_hdmi_soc: dw-hdmi-soc@0 {
+	compatible = "snps,dw-hdmi-soc";
+	reg = <0x11c00 0x1000>; /* EDIDs */
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	hdmi-rx@0 {
+		compatible = "snps,dw-hdmi-rx";
+		reg = <0x0 0x10000>;
+		interrupts = <1 2>;
+		edid-phandle = <&dw_hdmi_soc>;
+
+		clocks = <&dw_hdmi_refclk>;
+		clock-names = "cfg";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hdmi-phy@fc {
+			compatible = "snps,dw-hdmi-phy-e405";
+			reg = <0xfc>;
+
+			clocks = <&dw_hdmi_refclk>;
+			clock-names = "cfg";
+		};
+	};
+};
Signature · 7 lines
1.9.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#8 lore raw

Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX

Quoted 8 lines · Jose Abreu

Document the bindings for the Synopsys Designware HDMI RX.

---
  .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  | 70 ++++++++++++++++++++++
  1 file changed, 70 insertions(+)

create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt

Could you make the DT binding documentation patch first patch in the series? Now checkpatch will complain about undocumented compatible string when the driver patches are applied alone.

Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +0 -0
new file mode 100644
index 0000000..449b8a2

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 10

Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10 -0
@@ -0,0 +1,70 @@
+Synopsys DesignWare HDMI RX Decoder
+===================================
+
+This document defines device tree properties for the Synopsys DesignWare HDMI
+RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding
+specification by itself but is meant to be referenced by platform-specific
+device tree bindings.
+
+When referenced from platform device tree bindings the properties defined in
+this document are defined as follows.

It would be good to make it clear which properties are required and which are optional. And also to mention the properties below belong to the HDMI RX node.

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 16

Quoted 5 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5 -0
+- compatible: Shall be "snps,dw-hdmi-rx".
+
+- reg: Memory mapped base address and length of the DWC HDMI RX registers.
+
+- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt.

s/5v/HDMI 5V ?

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 22

Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6 -0
+
+- clocks: Phandle to the config clock block.
+
+- clock-names: Shall be "cfg".
+
+- edid-phandle: phandle to the EDID handler block.

Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 28

Quoted 5 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5 -0
+- #address-cells: Shall be 1.
+
+- #size-cells: Shall be 0.
+
+You also have to create a subnode for phy driver. Phy properties are as follows.

s/phy driver. Phy/the PHY device. PHY ?

Might be also worth to make it explicit these are all required properties.

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 32

Quoted 3 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +3 -0
+- compatible: Shall be "snps,dw-hdmi-phy-e405".
+
+- reg: Shall be JTAG address of phy.

s/phy/the PHY ?

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44

Quoted 11 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +11 -0
+- clocks: Phandle for cfg clock.
+
+- clock-names:Shall be "cfg".
+
+A sample binding is now provided. The compatible string is for a SoC which has
+has a Synopsys DesignWare HDMI RX decoder inside.
+
+Example:
+
+dw_hdmi_soc: dw-hdmi-soc@0 {
+	compatible = "snps,dw-hdmi-soc";

Perhaps just make it

	compatible = "...";

?

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 45

Quoted 1 line
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +1 -0
+	reg = <0x11c00 0x1000>; /* EDIDs */

This is not relevant and undocumented, will likely be part of documentation of other binding thus I'd suggest dropping this reg property.

Quoted 15 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +15 -0
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	hdmi-rx@0 {
+		compatible = "snps,dw-hdmi-rx";
+		reg = <0x0 0x10000>;
+		interrupts = <1 2>;
+		edid-phandle = <&dw_hdmi_soc>;
+
+		clocks = <&dw_hdmi_refclk>;
+		clock-names = "cfg";
+
+		#address-cells = <1>;
+		#size-cells = <0>;

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 70

Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10 -0
+
+		hdmi-phy@fc {
+			compatible = "snps,dw-hdmi-phy-e405";
+			reg = <0xfc>;
+
+			clocks = <&dw_hdmi_refclk>;
+			clock-names = "cfg";
+		};
+	};
+};

Otherwise looks good. I'll likely not have comments to the other patches.

--
Regards,
Sylwester

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#9 lore raw

Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX

Hi Sylwester,

Quoted 11 lines · Sylwester Nawrocki
Quoted 7 lines · Jose Abreu

Document the bindings for the Synopsys Designware HDMI RX.

---
  .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  | 70 ++++++++++++++++++++++
  1 file changed, 70 insertions(+)

create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt

Could you make the DT binding documentation patch first patch in the series? Now checkpatch will complain about undocumented compatible string when the driver patches are applied alone.

Sure.

Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +4 -0
new file mode 100644
index 0000000..449b8a2
@@ -0,0 +1,70 @@
+Synopsys DesignWare HDMI RX Decoder
+===================================
+
+This document defines device tree properties for the Synopsys DesignWare HDMI
Quoted 8 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 10

Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6 -0
+RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding
+specification by itself but is meant to be referenced by platform-specific
+device tree bindings.
+
+When referenced from platform device tree bindings the properties defined in
+this document are defined as follows.

It would be good to make it clear which properties are required and which are optional. And also to mention the properties below belong to the HDMI RX node.

Ok.

Quoted 7 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 16

Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5 -0
+- compatible: Shall be "snps,dw-hdmi-rx".
+
+- reg: Memory mapped base address and length of the DWC HDMI RX registers.
+
+- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt.

s/5v/HDMI 5V ?

Ok.

Quoted 10 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 22

Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6 -0
+
+- clocks: Phandle to the config clock block.
+
+- clock-names: Shall be "cfg".
+
+- edid-phandle: phandle to the EDID handler block.

Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.

Hmm, this is for the CEC notifier. Do you mean I should grab the parent device for the notifier? This property is already optional if cec is not enabled though.

Quoted 9 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 28

Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5 -0
+- #address-cells: Shall be 1.
+
+- #size-cells: Shall be 0.
+
+You also have to create a subnode for phy driver. Phy properties are as follows.

s/phy driver. Phy/the PHY device. PHY ?

Might be also worth to make it explicit these are all required properties.

Ok.

Quoted 5 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 32

Quoted 4 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +3 -0
+- compatible: Shall be "snps,dw-hdmi-phy-e405".
+
+- reg: Shall be JTAG address of phy.

s/phy/the PHY ?

Ok.

Quoted 14 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44

Quoted 12 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +11 -0
+- clocks: Phandle for cfg clock.
+
+- clock-names:Shall be "cfg".
+
+A sample binding is now provided. The compatible string is for a SoC which has
+has a Synopsys DesignWare HDMI RX decoder inside.
+
+Example:
+
+dw_hdmi_soc: dw-hdmi-soc@0 {
+	compatible = "snps,dw-hdmi-soc";

Perhaps just make it

Quoted 2 lines
	compatible = "...";

?

Yeah, probably its better.

Quoted 4 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 45

Quoted 2 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +1 -0
+	reg = <0x11c00 0x1000>; /* EDIDs */

This is not relevant and undocumented, will likely be part of documentation of other binding thus I'd suggest dropping this reg property.

Ok.

Quoted 19 lines
Quoted 19 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +18 -0
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges;
+
+	hdmi-rx@0 {
+		compatible = "snps,dw-hdmi-rx";
+		reg = <0x0 0x10000>;
+		interrupts = <1 2>;
+		edid-phandle = <&dw_hdmi_soc>;
+
+		clocks = <&dw_hdmi_refclk>;
+		clock-names = "cfg";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hdmi-phy@fc {
+			compatible = "snps,dw-hdmi-phy-e405";
Quoted 8 lines

Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 70

Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +7 -0
+			reg = <0xfc>;
+
+			clocks = <&dw_hdmi_refclk>;
+			clock-names = "cfg";
+		};
+	};
+};

Otherwise looks good. I'll likely not have comments to the other patches.

Thanks for the review!

Best regards,
Jose Miguel Abreu

Quoted 5 lines

--
Regards,
Sylwester

#10 lore raw

Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX

Quoted 9 lines · Jose Abreu
Quoted 5 lines
Quoted 1 line

+- edid-phandle: phandle to the EDID handler block.

Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.

Hmm, this is for the CEC notifier. Do you mean I should grab the parent device for the notifier? This property is already optional if cec is not enabled though.

Yes, device associated with the parent node. Something like:

- edid-phandle - phandle to the EDID handler block; if this property is not specified it is assumed that EDID is handled by device described by parent node of the HDMI RX node

Not sure if it is any better than always requiring edid-phandle property, even when it is pointing to the parent node. We would need a DT maintainer's opinion on that.

--
Thanks,
Sylwester

author
#11 lore raw

Re: [PATCH v6 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX

Quoted 11 lines · Sylwester Nawrocki
Quoted 7 lines · Jose Abreu
Quoted 4 lines
Quoted 1 line

+- edid-phandle: phandle to the EDID handler block.

Could you make this property optional and when it is missing assume that device corresponding to the parent node of this node handles EDID? This way we could avoid having property pointing to the parent node.

Hmm, this is for the CEC notifier. Do you mean I should grab the parent device for the notifier? This property is already optional if cec is not enabled though.

Yes, device associated with the parent node. Something like:

Quoted 7 lines

- edid-phandle - phandle to the EDID handler block; if this property is not specified it is assumed that EDID is handled by device described by parent node of the HDMI RX node

Not sure if it is any better than always requiring edid-phandle property, even when it is pointing to the parent node. We would need a DT maintainer's opinion on that.

I will change and resend. I also have to fix a kbuild error when cec is not enabled.

Best regards,
Jose Miguel Abreu

Quoted 4 lines

--
Thanks,
Sylwester