Synopsys Designware HDMI Video Capture Controller + PHY
Subsystems (2)
v4 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 4 addresses review comments from Sylwester Nawrocki that were mainly regarding the phy initialization and bindings in the DT. We switched to V4L2 async API so that we don't have to wait for phy to be initialized.
This series was tested in a FPGA platform.
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
.../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 | 832 +++++++++ drivers/media/platform/dwc/dw-hdmi-phy-e405.h | 63 + drivers/media/platform/dwc/dw-hdmi-rx.c | 1862 ++++++++++++++++++++ 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 | 97 + 12 files changed, 3529 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 · 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
PATCH v4 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.
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 | 832 ++++++++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-phy-e405.h | 63 ++ include/media/dwc/dw-hdmi-phy-pdata.h | 128 ++++ 7 files changed, 1036 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
@@ -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
@@ -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
@@ -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
@@ -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 +832 -0
832 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
Re: [PATCH v4 1/4] [media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver
Hi Jose,
Thank you for addressing my review comments. Couple more suggestions below.
Quoted 4 lines · Jose Abreu
This adds support for the Synopsys Designware HDMI RX PHY e405. This phy receives and decodes HDMI video that is delivered to a controller.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +1
-0+static int dw_phy_config(struct dw_phy_dev *dw_dev, unsigned char color_depth,
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 599
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +9
-0+ bool hdmi2, bool scrambling) +{ +phy_reset (dw_dev,1 ); +phy_pddq (dw_dev,1 ); +phy_svsmode (dw_dev,1 ); + +phy_zcal_reset (dw_dev); + do { + udelay(1000);
Could be mdelay(1) or better e.g. usleep_range(1000, 1100);
Quoted 12 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+ zcal_done =phy_zcal_done (dw_dev); + }while (!zcal_done && timeout--); + +if (!zcal_done) { +dev_err (dw_dev->dev,"Zcal calibration failed\n" ); +return -ETIMEDOUT ; + } +return 0 ; +} +static int dw_phy_probe (struct platform_device *pdev)
drivers/media/platform/dwc/dw-hdmi-phy-e405.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+{ +struct dw_phy_pdata *pdata = pdev->dev.platform_data; +struct device *dev = &pdev->dev; +struct dw_phy_dev *dw_dev; +struct v4l2_subdev *sd; +int ret; + +dev_dbg (dev,"probe start\n" ); + +/* Resource allocation */
This comment is not needed.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 748
Quoted 5 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +5
-0+ dw_dev =devm_kzalloc (dev,sizeof (*dw_dev),GFP_KERNEL ); +if (!dw_dev) +return -ENOMEM ; + +/* Resource initialization */
Ditto.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 756
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +8
-0+if (!pdata) { +dev_err (dev,"no platform data suplied\n" ); +return -EINVAL ; + } + + dw_dev->dev = dev; + dw_dev->config = pdata; + dw_dev->version =405 ;
How about storing the version number in the dw_hdmi_phy_e405_id[] table and retrieving it here with of_device_get_match_data() ?
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 759
Quoted 3 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +3
-0+mutex_init (&dw_dev->lock); + +/* Get config clock value */
The comment is not needed, it's clear from the code we are getting the clock and its rate.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 760
Quoted 1 line
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +1
-0+ dw_dev->clk =devm_clk_get (dev,"cfg-clk" );
As Rob suggested, it would be good to change name of the clock to just "cfg".
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 772
Quoted 12 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +12
-0+if (IS_ERR (dw_dev->clk)) { +dev_err (dev,"failed to get cfg-clk\n" ); +return PTR_ERR (dw_dev->clk); + } + + ret =clk_prepare_enable (dw_dev->clk); +if (ret) { +dev_err (dev,"failed to enable cfg-clk\n" ); +return ret; + } + + dw_dev->cfg_clk =clk_get_rate (dw_dev->clk) /1000000 ;
1000000U ?
Quoted 13 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +13
-0+if (!dw_dev->cfg_clk) { + dev_err(dev, "invalid cfg-clk frequency\n");> + ret = -EINVAL; + goto err_clk; + } + +/* V4L2 initialization */ + sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_phy_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + +/* Force phy disabling */ + dw_dev->phy_enabled = true;
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 796
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+dw_phy_disable (dw_dev); + +/* Register subdev */ + ret =v4l2_async_register_subdev (sd); +if (ret) { +dev_err (dev,"failed to register subdev\n" ); + goto err_clk; + } + +/* All done */
Superfluous comment.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 798
Quoted 2 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +2
-0+dev_set_drvdata (dev, sd); +dev_info (dev,"driver probed (cfg-clk=%d)\n" , dw_dev->cfg_clk);
This should be at dev_dbg() level.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 808
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+return 0 ; + +err_clk: +clk_disable_unprepare (dw_dev->clk); +return ret; +} + +static int dw_phy_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev;
The assignment here could be dropped and &pdev->dev used directly below.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 814
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +6
-0+struct v4l2_subdev *sd =dev_get_drvdata (dev); +struct dw_phy_dev *dw_dev =to_dw_dev (sd); + +v4l2_async_unregister_subdev (sd); +clk_disable_unprepare (dw_dev->clk); +dev_info (dev,"driver removed\n" );
This should be at dev_dbg() level or dropped entirely.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 822
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +8
-0+return 0 ; +} + +static const struct of_device_id dw_hdmi_phy_e405_id[] = { + { .compatible ="snps,dw-hdmi-phy-e405" }, + { }, +}; +MODULE_DEVICE_TABLE (of, dw_hdmi_phy_e405_id);
--
Regards,
Sylwester
Re: [PATCH v4 1/4] [media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver
Hi Sylwester,
Quoted 5 lines · Sylwester Nawrocki
Hi Jose,
Thank you for addressing my review comments. Couple more suggestions below.
On 06/20/2017 07:26 PM, Jose Abreu wrote:
Quoted 13 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 599
Quoted 12 lines
This adds support for the Synopsys Designware HDMI RX PHY e405. This phy receives and decodes HDMI video that is delivered to a controller.
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+static int dw_phy_config (struct dw_phy_dev *dw_dev,unsigned char color_depth, +bool hdmi2,bool scrambling) +{ +phy_reset (dw_dev,1 ); +phy_pddq (dw_dev,1 ); +phy_svsmode (dw_dev,1 ); + +phy_zcal_reset (dw_dev); + do { + udelay(1000);Could be mdelay(1) or better e.g.
usleep_range(1000, 1100);
Ok.
Quoted 23 lines
Quoted 11 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+ zcal_done =phy_zcal_done (dw_dev); + }while (!zcal_done && timeout--); + +if (!zcal_done) { +dev_err (dw_dev->dev,"Zcal calibration failed\n" ); +return -ETIMEDOUT ; + } +return 0 ; +} +static int dw_phy_probe (struct platform_device *pdev)drivers/media/platform/dwc/dw-hdmi-phy-e405.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+{ +struct dw_phy_pdata *pdata = pdev->dev.platform_data; +struct device *dev = &pdev->dev; +struct dw_phy_dev *dw_dev; +struct v4l2_subdev *sd; +int ret; + +dev_dbg (dev,"probe start\n" ); + +/* Resource allocation */ This comment is not needed.
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 748
Quoted 5 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +5
-0+ dw_dev =devm_kzalloc (dev,sizeof (*dw_dev),GFP_KERNEL ); +if (!dw_dev) +return -ENOMEM ; + +/* Resource initialization */ Ditto.
Ok.
Quoted 11 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 756
Quoted 9 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +8
-0+if (!pdata) { +dev_err (dev,"no platform data suplied\n" ); +return -EINVAL ; + } + + dw_dev->dev = dev; + dw_dev->config = pdata; + dw_dev->version =405 ;How about storing the version number in the dw_hdmi_phy_e405_id[] table and retrieving it here with
of_device_get_match_data()?
Yeah, seems nice :)
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 759
Quoted 4 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +3
-0+mutex_init (&dw_dev->lock); + +/* Get config clock value */ The comment is not needed, it's clear from the code we are getting the clock and its rate.
Ok.
Quoted 3 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 760
Quoted 2 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +1
-0+ dw_dev->clk =devm_clk_get (dev,"cfg-clk" );As Rob suggested, it would be good to change name of the clock to just "cfg".
Yes, I will change.
Quoted 14 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 772
Quoted 13 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +12
-0+if (IS_ERR (dw_dev->clk)) { +dev_err (dev,"failed to get cfg-clk\n" ); +return PTR_ERR (dw_dev->clk); + } + + ret =clk_prepare_enable (dw_dev->clk); +if (ret) { +dev_err (dev,"failed to enable cfg-clk\n" ); +return ret; + } + + dw_dev->cfg_clk =clk_get_rate (dw_dev->clk) /1000000 ;1000000U ?
Ok.
Quoted 17 lines
Quoted 17 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +16
-0+if (!dw_dev->cfg_clk) { + dev_err(dev, "invalid cfg-clk frequency\n");> + ret = -EINVAL; + goto err_clk; + } + +/* V4L2 initialization */ + sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_phy_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + +/* Force phy disabling */ + dw_dev->phy_enabled = true; +dw_phy_disable (dw_dev); + +/* Register subdev */
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 796
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +7
-0+ ret =v4l2_async_register_subdev (sd); +if (ret) { +dev_err (dev,"failed to register subdev\n" ); + goto err_clk; + } + +/* All done */ Superfluous comment.
Ok.
Quoted 4 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 798
Quoted 3 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +2
-0+dev_set_drvdata (dev, sd); +dev_info (dev,"driver probed (cfg-clk=%d)\n" , dw_dev->cfg_clk);This should be at
dev_dbg()level.
Ok.
Quoted 12 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 808
Quoted 11 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +10
-0+return 0 ; + +err_clk: +clk_disable_unprepare (dw_dev->clk); +return ret; +} + +static int dw_phy_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev;The assignment here could be dropped and
&pdev->devused directly below.
Ok.
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 814
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +6
-0+struct v4l2_subdev *sd =dev_get_drvdata (dev); +struct dw_phy_dev *dw_dev =to_dw_dev (sd); + +v4l2_async_unregister_subdev (sd); +clk_disable_unprepare (dw_dev->clk); +dev_info (dev,"driver removed\n" );This should be at
dev_dbg()level or dropped entirely.
Ok.
Thanks for the review!
Best regards,
Jose Miguel Abreu
Quoted 12 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c line 822
Quoted 9 lines
drivers/media/platform/dwc/dw-hdmi-phy-e405.c new file +8
-0+return 0 ; +} + +static const struct of_device_id dw_hdmi_phy_e405_id[] = { + { .compatible ="snps,dw-hdmi-phy-e405" }, + { }, +}; +MODULE_DEVICE_TABLE (of, dw_hdmi_phy_e405_id);--
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
PATCH v4 2/4 platform: Add Synopsys Designware HDMI RX Controller Driver
In reply to Jose AbreuThis 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).
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 | 1862 +++++++++++++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-rx.h | 441 ++++++++ include/media/dwc/dw-hdmi-rx-pdata.h | 97 ++ 5 files changed, 2416 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
@@ -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
@@ -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 +1862 -0
1,862 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 +97 -0
97 lines not on this page · raw message
Signature · 1 line
1.9.1
Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Quoted 71 lines · 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.
Cc Carlos Palminha <palminha-HKixBCOQz3hWk0Htik3J/[email protected]>Cc Mauro Carvalho Chehab <[email protected]>Cc Hans Verkuil <[email protected]>Cc Sylwester Nawrocki <[email protected]>Changes from v3:
- Use v4l2 async API (Sylwester)
- Do not block waiting for phy
- Do not use busy waiting delays (Sylwester)
- Simplifydw_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
- FixupHDMI_VICnot 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 | 1862 +++++++++++++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-rx.h | 441 ++++++++ include/media/dwc/dw-hdmi-rx-pdata.h | 97 ++ 5 files changed, 2416 insertions(+)create mode 100644
drivers/media/platform/dwc/dw-hdmi-rx.ccreate mode 100644drivers/media/platform/dwc/dw-hdmi-rx.hcreate mode 100644include/media/dwc/dw-hdmi-rx-pdata.hdrivers/media/platform/dwc/dw-hdmi-rx.c new file +18
-0+static const struct of_device_id dw_hdmi_supported_phys[] = { + { .compatible ="snps,dw-hdmi-phy-e405" , .data =DW_PHY_E405_DRVNAME , }, + { }, +}; + +static struct device_node *dw_hdmi_get_phy_of_node (struct dw_hdmi_dev *dw_dev, +const struct of_device_id **found_id) +{ +struct device_node *child =NULL ; +const struct of_device_id *id; + +for_each_child_of_node (dw_dev->of_node, child) { + id =of_match_node (dw_hdmi_supported_phys, child); +if (id) +break ; + } + +if (found_id)
drivers/media/platform/dwc/dw-hdmi-rx.c line 439
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+ *found_id = id; + +return child; +} + +static int dw_hdmi_phy_init (struct dw_hdmi_dev *dw_dev) +{ +struct dw_phy_pdata *phy = &dw_dev->phy_config; +const struct of_device_id *of_id; +struct of_dev_auxdata lookup;
struct of_dev_auxdata lookup = { };You could initialize to 0 here and...
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+struct device_node *child; +const char *drvname; +int ret; + + child =dw_hdmi_get_phy_of_node (dw_dev, &of_id); +if (!child || !of_id || !of_id->data) { +dev_err (dw_dev->dev,"no supported phy found in DT\n" );
drivers/media/platform/dwc/dw-hdmi-rx.c line 456
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+return -EINVAL ; + } + + drvname = of_id->data; + phy->funcs = &dw_hdmi_phy_funcs; + phy->funcs_arg = dw_dev; + + lookup.compatible = (char *)of_id->compatible; + lookup.phys_addr =0x0 ; + lookup.name =NULL ;
...drop these two assignments.
drivers/media/platform/dwc/dw-hdmi-rx.c line 459
Quoted 3 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +3
-0+ lookup.platform_data = phy; + +request_module (drvname);
I'd say this request_module() is not needed when you use the v4l2-async subnotifiers and the modules are properly installed in the file system. I might be missing something though.
Quoted 67 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +65
-0+ ret =of_platform_populate (dw_dev->of_node,NULL , &lookup, dw_dev->dev); +if (ret) { +dev_err (dw_dev->dev,"failed to populate phy driver\n" ); +return ret; + } + +return 0 ; +} + +static void dw_hdmi_phy_exit (struct dw_hdmi_dev *dw_dev) +{ +of_platform_depopulate (dw_dev->dev); +} +static int dw_hdmi_registered (struct v4l2_subdev *sd) +{ +struct dw_hdmi_dev *dw_dev =to_dw_dev (sd); +int ret; + + ret =cec_register_adapter (dw_dev->cec_adap, dw_dev->dev); +if (ret) { +dev_err (dw_dev->dev,"failed to register CEC adapter\n" ); +cec_delete_adapter (dw_dev->cec_adap); +return ret; + } + +cec_register_cec_notifier (dw_dev->cec_adap, dw_dev->cec_notifier); + dw_dev->registered = true; +return ret; +} + +static void dw_hdmi_unregistered (struct v4l2_subdev *sd) +{ +struct dw_hdmi_dev *dw_dev =to_dw_dev (sd); + +cec_unregister_adapter (dw_dev->cec_adap); +cec_notifier_put (dw_dev->cec_notifier); +} + +static const struct v4l2_subdev_internal_ops dw_hdmi_internal_ops = { + .registered = dw_hdmi_registered, + .unregistered = dw_hdmi_unregistered, +}; + +static int dw_hdmi_v4l2_notify_bound (struct v4l2_async_notifier *notifier, +struct v4l2_subdev *subdev,struct v4l2_async_subdev *asd) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); + +if (dw_dev->phy_async_sd.match.fwnode.fwnode == +of_fwnode_handle (subdev->dev->of_node)) { +dev_dbg (dw_dev->dev,"found new subdev '%s'\n" , subdev->name); + dw_dev->phy_sd = subdev; +return 0 ; + } + +return -EINVAL ; +} + +static void dw_hdmi_v4l2_notify_unbind (struct v4l2_async_notifier *notifier, +struct v4l2_subdev *subdev,struct v4l2_async_subdev *asd) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); + +if (dw_dev->phy_sd == subdev) { +dev_dbg (dw_dev->dev,"unbinding '%s'\n" , subdev->name);
drivers/media/platform/dwc/dw-hdmi-rx.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+ dw_dev->phy_sd =NULL ; + } +} + +static int dw_hdmi_v4l2_notify_complete (struct v4l2_async_notifier *notifier) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); +int ret; + + ret =v4l2_device_register_subdev_nodes (&dw_dev->v4l2_dev);
There shouldn't be multiple struct v4l2_device instances, instead we should have only one created by the main driver. AFAIU, in your case it would be driver associated with the dw-hdmi-soc DT node. And normally such a top level driver creates subdev device nodes when its all required sub-devices are available.
I think this patch could be useful for you:
https://patchwork.linuxtv.org/patch/41834
With that the dw-hdmi-soc driver would have it's v4l2-async notifier's notify_complete() callback called only when both the hdmi-rx and the hdmi-phy subdevs are registered.
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+if (ret) { +dev_err (dw_dev->dev,"failed to register subdev nodes\n" ); +return ret; + } + +return 0 ; +}
drivers/media/platform/dwc/dw-hdmi-rx.c line 1631
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+static int dw_hdmi_parse_dt (struct dw_hdmi_dev *dw_dev) +{ + ret =clk_prepare_enable (dw_dev->clk); +if (ret) { +dev_err (dw_dev->dev,"failed to enable cfg-clk\n" ); +return ret; + } + + dw_dev->cfg_clk =clk_get_rate (dw_dev->clk) /1000000 ;
1000000U ?
Quoted 14 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +13
-0+if (!dw_dev->cfg_clk) { +dev_err (dw_dev->dev,"invalid cfg-clk frequency\n" ); + ret = -EINVAL ; + goto err_clk; + } + +return 0 ; + +err_clk: +clk_disable_unprepare (dw_dev->clk); +return ret; +} +
drivers/media/platform/dwc/dw-hdmi-rx.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+static int dw_hdmi_rx_probe (struct platform_device *pdev) +{ +/* V4L2 initialization */ + sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_hdmi_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + sd->internal_ops = &dw_hdmi_internal_ops; + sd->flags |=V4L2_SUBDEV_FL_HAS_EVENTS ;
Don't you also need V4L2_SUBDEV_FL_HAS_DEVNODE flag set?
drivers/media/platform/dwc/dw-hdmi-rx.c line 1766
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +8
-0+/* Notifier for subdev binding */ + ret =dw_hdmi_v4l2_init_notifier (dw_dev); +if (ret) { +dev_err (dev,"failed to init v4l2 notifier\n" ); + goto err_v4l2_dev; + } + +/* Phy loading */
s/Phy/PHY/ ?
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +5
-0+ ret =dw_hdmi_phy_init (dw_dev); +if (ret) + goto err_v4l2_notifier; + + ret =v4l2_async_register_subdev (sd);
drivers/media/platform/dwc/dw-hdmi-rx.c line 1805
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+if (ret) { +dev_err (dev,"failed to register subdev\n" ); + goto err_cec; + } + +/* Fill initial format settings */ + dw_dev->timings = timings_def; + dw_dev->mbus_code =MEDIA_BUS_FMT_BGR888_1X24 ; + +/* All done */
This comment seems unneeded.
drivers/media/platform/dwc/dw-hdmi-rx.c line 1809
Quoted 5 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +4
-0+dev_set_drvdata (dev, sd); + dw_dev->state =HDMI_STATE_POWER_OFF ; +dw_hdmi_detect_tx_5v (dw_dev); +dev_info (dev,"driver probed\n" );
dev_dbg()
Quoted 26 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +26
-0+return 0 ; + +err_cec: +cec_delete_adapter (dw_dev->cec_adap); +err_phy: +dw_hdmi_phy_exit (dw_dev); +err_v4l2_notifier: +v4l2_async_notifier_unregister (&dw_dev->v4l2_notifier); +err_v4l2_dev: +v4l2_device_unregister (&dw_dev->v4l2_dev); +err_wq: +destroy_workqueue (dw_dev->wq); +return ret; +} + +static int dw_hdmi_rx_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct v4l2_subdev *sd =dev_get_drvdata (dev); +struct dw_hdmi_dev *dw_dev =to_dw_dev (sd); + +dev_dbg (dev,"%s\n" , __func__); + +dw_hdmi_disable_ints (dw_dev); +dw_hdmi_disable_hpd (dw_dev); +dw_hdmi_disable_scdc (dw_dev);
drivers/media/platform/dwc/dw-hdmi-rx.c line 1844
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+dw_hdmi_power_off (dw_dev); +dw_hdmi_phy_s_power (dw_dev, false); +flush_workqueue (dw_dev->wq); +destroy_workqueue (dw_dev->wq); +v4l2_device_unregister (&dw_dev->v4l2_dev); +dw_hdmi_phy_exit (dw_dev); +v4l2_async_unregister_subdev (sd); +clk_disable_unprepare (dw_dev->clk); +dev_info (dev,"driver removed\n" );
dev_dbg()
drivers/media/platform/dwc/dw-hdmi-rx.c line 1852
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +8
-0+return 0 ; +} + +static const struct of_device_id dw_hdmi_rx_id[] = { + { .compatible ="snps,dw-hdmi-rx" }, + { }, +}; +MODULE_DEVICE_TABLE (of, dw_hdmi_rx_id);
--
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
Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Hi Sylwester,
Quoted 93 lines · Sylwester Nawrocki
Quoted 68 lines · 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.
Signed-off-by Jose Abreu <[email protected]>Cc Carlos Palminha <[email protected]>Cc Mauro Carvalho Chehab <[email protected]>Cc Hans Verkuil <[email protected]>Cc Sylwester Nawrocki <[email protected]>Changes from v3:
- Use v4l2 async API (Sylwester)
- Do not block waiting for phy
- Do not use busy waiting delays (Sylwester)
- Simplifydw_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
- FixupHDMI_VICnot 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 | 1862 +++++++++++++++++++++++++++++++ drivers/media/platform/dwc/dw-hdmi-rx.h | 441 ++++++++ include/media/dwc/dw-hdmi-rx-pdata.h | 97 ++ 5 files changed, 2416 insertions(+)create mode 100644
drivers/media/platform/dwc/dw-hdmi-rx.ccreate mode 100644drivers/media/platform/dwc/dw-hdmi-rx.hcreate mode 100644include/media/dwc/dw-hdmi-rx-pdata.hdrivers/media/platform/dwc/dw-hdmi-rx.c new file +18
-0+static const struct of_device_id dw_hdmi_supported_phys[] = { + { .compatible ="snps,dw-hdmi-phy-e405" , .data =DW_PHY_E405_DRVNAME , }, + { }, +}; + +static struct device_node *dw_hdmi_get_phy_of_node (struct dw_hdmi_dev *dw_dev, +const struct of_device_id **found_id) +{ +struct device_node *child =NULL ; +const struct of_device_id *id; + +for_each_child_of_node (dw_dev->of_node, child) { + id =of_match_node (dw_hdmi_supported_phys, child); +if (id) +break ; + } + +if (found_id)drivers/media/platform/dwc/dw-hdmi-rx.c line 439
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+ *found_id = id; + +return child; +} + +static int dw_hdmi_phy_init (struct dw_hdmi_dev *dw_dev) +{ +struct dw_phy_pdata *phy = &dw_dev->phy_config; +const struct of_device_id *of_id; +struct of_dev_auxdata lookup;struct of_dev_auxdata lookup = { };You could initialize to 0 here and...
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+struct device_node *child; +const char *drvname; +int ret; + + child =dw_hdmi_get_phy_of_node (dw_dev, &of_id); +if (!child || !of_id || !of_id->data) { +dev_err (dw_dev->dev,"no supported phy found in DT\n" ); +return -EINVAL ; + } +
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 456
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+ drvname = of_id->data; + phy->funcs = &dw_hdmi_phy_funcs; + phy->funcs_arg = dw_dev; + + lookup.compatible = (char *)of_id->compatible; + lookup.phys_addr =0x0 ; + lookup.name =NULL ;...drop these two assignments.
Ok.
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 459
Quoted 4 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +3
-0+ lookup.platform_data = phy; + +request_module (drvname);I'd say this
request_module()is not needed when you use the v4l2-async subnotifiers and the modules are properly installed in the file system. I might be missing something though.
Hmm, well I didn't actually test without request_module but I think its needed, otherwise I would have to do:
modprobe phy_module
modprobe controller_module
With request_module I just have to do:
modprobe controller_module
Quoted 82 lines
Quoted 66 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +65
-0+ ret =of_platform_populate (dw_dev->of_node,NULL , &lookup, dw_dev->dev); +if (ret) { +dev_err (dw_dev->dev,"failed to populate phy driver\n" ); +return ret; + } + +return 0 ; +} + +static void dw_hdmi_phy_exit (struct dw_hdmi_dev *dw_dev) +{ +of_platform_depopulate (dw_dev->dev); +} +static int dw_hdmi_registered (struct v4l2_subdev *sd) +{ +struct dw_hdmi_dev *dw_dev =to_dw_dev (sd); +int ret; + + ret =cec_register_adapter (dw_dev->cec_adap, dw_dev->dev); +if (ret) { +dev_err (dw_dev->dev,"failed to register CEC adapter\n" ); +cec_delete_adapter (dw_dev->cec_adap); +return ret; + } + +cec_register_cec_notifier (dw_dev->cec_adap, dw_dev->cec_notifier); + dw_dev->registered = true; +return ret; +} + +static void dw_hdmi_unregistered (struct v4l2_subdev *sd) +{ +struct dw_hdmi_dev *dw_dev =to_dw_dev (sd); + +cec_unregister_adapter (dw_dev->cec_adap); +cec_notifier_put (dw_dev->cec_notifier); +} + +static const struct v4l2_subdev_internal_ops dw_hdmi_internal_ops = { + .registered = dw_hdmi_registered, + .unregistered = dw_hdmi_unregistered, +}; + +static int dw_hdmi_v4l2_notify_bound (struct v4l2_async_notifier *notifier, +struct v4l2_subdev *subdev,struct v4l2_async_subdev *asd) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); + +if (dw_dev->phy_async_sd.match.fwnode.fwnode == +of_fwnode_handle (subdev->dev->of_node)) { +dev_dbg (dw_dev->dev,"found new subdev '%s'\n" , subdev->name); + dw_dev->phy_sd = subdev; +return 0 ; + } + +return -EINVAL ; +} + +static void dw_hdmi_v4l2_notify_unbind (struct v4l2_async_notifier *notifier, +struct v4l2_subdev *subdev,struct v4l2_async_subdev *asd) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); + +if (dw_dev->phy_sd == subdev) { +dev_dbg (dw_dev->dev,"unbinding '%s'\n" , subdev->name);drivers/media/platform/dwc/dw-hdmi-rx.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+ dw_dev->phy_sd =NULL ; + } +} + +static int dw_hdmi_v4l2_notify_complete (struct v4l2_async_notifier *notifier) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); +int ret; + + ret =v4l2_device_register_subdev_nodes (&dw_dev->v4l2_dev);There shouldn't be multiple struct
v4l2_deviceinstances, instead we should have only one created by the main driver. AFAIU, in your case it would be driver associated with the dw-hdmi-soc DT node. And normally such a top level driver creates subdev device nodes when its all required sub-devices are available.
Quoted 6 lines
I think this patch could be useful for you:
https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.linuxtv.org_patch_41834&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=VfuOK5-A8h_JQk0lk74o48aav0T2PnwxhIr2sauw46Y&s=0GuG3FNbkmfVVMYQuP-Ed6zY9PnXnw7S4KgQhG52JF4&e=With that the dw-hdmi-soc driver would have it's v4l2-async notifier's
notify_complete()callback called only when both the hdmi-rx and the hdmi-phy subdevs are registered.
Yeah, I saw the patches. I just implemented this way because they are not merged yet, right?
Quoted 10 lines
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+if (ret) { +dev_err (dw_dev->dev,"failed to register subdev nodes\n" ); +return ret; + } + +return 0 ; +} +static int dw_hdmi_parse_dt (struct dw_hdmi_dev *dw_dev) +{
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1631
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+ ret =clk_prepare_enable (dw_dev->clk); +if (ret) { +dev_err (dw_dev->dev,"failed to enable cfg-clk\n" ); +return ret; + } + + dw_dev->cfg_clk =clk_get_rate (dw_dev->clk) /1000000 ;1000000U ?
Ok.
Quoted 16 lines
Quoted 16 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +15
-0+if (!dw_dev->cfg_clk) { +dev_err (dw_dev->dev,"invalid cfg-clk frequency\n" ); + ret = -EINVAL ; + goto err_clk; + } + +return 0 ; + +err_clk: +clk_disable_unprepare (dw_dev->clk); +return ret; +} + +static int dw_hdmi_rx_probe (struct platform_device *pdev) +{
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1748
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+/* V4L2 initialization */ + sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_hdmi_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + sd->internal_ops = &dw_hdmi_internal_ops; + sd->flags |=V4L2_SUBDEV_FL_HAS_EVENTS ;Don't you also need
V4L2_SUBDEV_FL_HAS_DEVNODEflag set?
Ouch. Yes I need otherwise the subdev will not be associated with the v4l2_device.
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1766
Quoted 9 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +8
-0+/* Notifier for subdev binding */ + ret =dw_hdmi_v4l2_init_notifier (dw_dev); +if (ret) { +dev_err (dev,"failed to init v4l2 notifier\n" ); + goto err_v4l2_dev; + } + +/* Phy loading */ s/Phy/PHY/ ?
Ok.
Quoted 17 lines
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +5
-0+ ret =dw_hdmi_phy_init (dw_dev); +if (ret) + goto err_v4l2_notifier; + + ret =v4l2_async_register_subdev (sd);drivers/media/platform/dwc/dw-hdmi-rx.c line 1805
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+if (ret) { +dev_err (dev,"failed to register subdev\n" ); + goto err_cec; + } + +/* Fill initial format settings */ + dw_dev->timings = timings_def; + dw_dev->mbus_code =MEDIA_BUS_FMT_BGR888_1X24 ; + +/* All done */ This comment seems unneeded.
Ok.
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1809
Quoted 5 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +4
-0+dev_set_drvdata (dev, sd); + dw_dev->state =HDMI_STATE_POWER_OFF ; +dw_hdmi_detect_tx_5v (dw_dev); +dev_info (dev,"driver probed\n" );
dev_dbg()
Ok.
Quoted 29 lines
Quoted 29 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +28
-0+return 0 ; + +err_cec: +cec_delete_adapter (dw_dev->cec_adap); +err_phy: +dw_hdmi_phy_exit (dw_dev); +err_v4l2_notifier: +v4l2_async_notifier_unregister (&dw_dev->v4l2_notifier); +err_v4l2_dev: +v4l2_device_unregister (&dw_dev->v4l2_dev); +err_wq: +destroy_workqueue (dw_dev->wq); +return ret; +} + +static int dw_hdmi_rx_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct v4l2_subdev *sd =dev_get_drvdata (dev); +struct dw_hdmi_dev *dw_dev =to_dw_dev (sd); + +dev_dbg (dev,"%s\n" , __func__); + +dw_hdmi_disable_ints (dw_dev); +dw_hdmi_disable_hpd (dw_dev); +dw_hdmi_disable_scdc (dw_dev); +dw_hdmi_power_off (dw_dev); +dw_hdmi_phy_s_power (dw_dev, false);
Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1844
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+flush_workqueue (dw_dev->wq); +destroy_workqueue (dw_dev->wq); +v4l2_device_unregister (&dw_dev->v4l2_dev); +dw_hdmi_phy_exit (dw_dev); +v4l2_async_unregister_subdev (sd); +clk_disable_unprepare (dw_dev->clk); +dev_info (dev,"driver removed\n" );
dev_dbg()
Ok.
Thanks for the review!
Best regards,
Jose Miguel Abreu
Quoted 12 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1852
Quoted 9 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +8
-0+return 0 ; +} + +static const struct of_device_id dw_hdmi_rx_id[] = { + { .compatible ="snps,dw-hdmi-rx" }, + { }, +}; +MODULE_DEVICE_TABLE (of, dw_hdmi_rx_id);--
Regards,
Sylwester
Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Hi Jose,
Quoted 50 lines · Jose Abreu
Quoted 27 lines · Sylwester Nawrocki
drivers/media/platform/dwc/dw-hdmi-rx.c line 439
Quoted 11 lines · 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.
drivers/media/platform/dwc/dw-hdmi-rx.c new file +5
-0+static int dw_hdmi_phy_init (struct dw_hdmi_dev *dw_dev) +{ +struct dw_phy_pdata *phy = &dw_dev->phy_config; +const struct of_device_id *of_id; +struct of_dev_auxdata lookup;struct of_dev_auxdata lookup = { };You could initialize to 0 here and...
Quoted 11 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +11
-0+struct device_node *child; +const char *drvname; +int ret; + + child =dw_hdmi_get_phy_of_node (dw_dev, &of_id); +if (!child || !of_id || !of_id->data) { +dev_err (dw_dev->dev,"no supported phy found in DT\n" ); +return -EINVAL ; + } + + drvname = of_id->data;Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 456
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +6
-0+ phy->funcs = &dw_hdmi_phy_funcs; + phy->funcs_arg = dw_dev; + + lookup.compatible = (char *)of_id->compatible; + lookup.phys_addr =0x0 ; + lookup.name =NULL ;...drop these two assignments.
Ok.
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 459
Quoted 3 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +3
-0+ lookup.platform_data = phy; + +request_module (drvname);I'd say this
request_module()is not needed when you use the v4l2-async subnotifiers and the modules are properly installed in the file system. I might be missing something though.Hmm, well I didn't actually test without
request_modulebut I think its needed, otherwise I would have to do:
Quoted 6 lines
modprobe
phy_module
modprobecontroller_moduleWith
request_moduleI just have to do:modprobe
controller_module
If you are sure you need it I'm not against that. But assuming you have udev in your system it should also work like this, without request_module():
1. modprobe controller_module -> phy device is created in the kernel, uevent sent
2. udev receives uevent, finds matching module and does modprobe phy_module
Remaining part is as before: phy_module registers the driver which gets matched with phy device; probe() is called which registers v4l2 subdev which then is registered to v4l2_device through the v4l2-async mechanism.
All this assumes udev is running and modules are installed in /lib/modules/$(uname -r). E.g. there should be your module alias as shown by modinfo phy_module in /lib/modules/$(uname -r)/modules.alias.
Quoted 30 lines
Quoted 30 lines
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+ ret =of_platform_populate (dw_dev->of_node,NULL , &lookup, dw_dev->dev); +if (ret) { +dev_err (dw_dev->dev,"failed to populate phy driver\n" ); +return ret; + } + +return 0 ; +} + +static void dw_hdmi_phy_exit (struct dw_hdmi_dev *dw_dev)drivers/media/platform/dwc/dw-hdmi-rx.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+{ +of_platform_depopulate (dw_dev->dev); +} +static int dw_hdmi_v4l2_notify_complete (struct v4l2_async_notifier *notifier) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); +int ret; + + ret =v4l2_device_register_subdev_nodes (&dw_dev->v4l2_dev);There shouldn't be multiple struct
v4l2_deviceinstances, instead we should have only one created by the main driver. AFAIU, in your case it would be driver associated with the dw-hdmi-soc DT node. And normally such a top level driver creates subdev device nodes when its all required sub-devices are available.I think this patch could be useful for you:
https://patchwork.linuxtv.org/patch/41834
Quoted 6 lines
Quoted 3 lines
With that the dw-hdmi-soc driver would have it's v4l2-async notifier's
notify_complete()callback called only when both the hdmi-rx and the hdmi-phy subdevs are registered.Yeah, I saw the patches. I just implemented this way because they are not merged yet, right?
I think these patches will be merged in v4.14-rc1, so together with your driver. You could apply them locally and indicate that your series depends on them in the cover letter.
Quoted 20 lines
Quoted 8 lines
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+if (ret) { +dev_err (dw_dev->dev,"failed to register subdev nodes\n" ); +return ret; + } + +return 0 ; +}Quoted 11 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1748
Quoted 9 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+static int dw_hdmi_rx_probe (struct platform_device *pdev) +{ +/* V4L2 initialization */ + sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_hdmi_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + sd->internal_ops = &dw_hdmi_internal_ops; + sd->flags |=V4L2_SUBDEV_FL_HAS_EVENTS ;Don't you also need
V4L2_SUBDEV_FL_HAS_DEVNODEflag set?
Quoted 2 lines
Ouch. Yes I need otherwise the subdev will not be associated with the
v4l2_device.
This flag indicates that the v4l2 subdev device node (/dev/v4l-subdev?) should be created for this subdevice.
---
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
Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Hi Sylwester,
Quoted 60 lines · Sylwester Nawrocki
Hi Jose,
Quoted 45 lines · Jose Abreu
Quoted 25 lines · Sylwester Nawrocki
drivers/media/platform/dwc/dw-hdmi-rx.c line 439
Quoted 10 lines · 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.
Signed-off-by Jose Abreu <[email protected]>drivers/media/platform/dwc/dw-hdmi-rx.c new file +5
-0+static int dw_hdmi_phy_init (struct dw_hdmi_dev *dw_dev) +{ +struct dw_phy_pdata *phy = &dw_dev->phy_config; +const struct of_device_id *of_id; +struct of_dev_auxdata lookup;struct of_dev_auxdata lookup = { };You could initialize to 0 here and...
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+struct device_node *child; +const char *drvname; +int ret; + + child =dw_hdmi_get_phy_of_node (dw_dev, &of_id); +if (!child || !of_id || !of_id->data) { +dev_err (dw_dev->dev,"no supported phy found in DT\n" ); +return -EINVAL ; + } +Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 456
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +7
-0+ drvname = of_id->data; + phy->funcs = &dw_hdmi_phy_funcs; + phy->funcs_arg = dw_dev; + + lookup.compatible = (char *)of_id->compatible; + lookup.phys_addr =0x0 ; + lookup.name =NULL ;...drop these two assignments.
Ok.
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 459
Quoted 3 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +3
-0+ lookup.platform_data = phy; + +request_module (drvname);I'd say this
request_module()is not needed when you use the v4l2-async subnotifiers and the modules are properly installed in the file system. I might be missing something though.Hmm, well I didn't actually test without
request_modulebut I think its needed, otherwise I would have to do:Quoted 6 lines
modprobe
phy_module
modprobecontroller_moduleWith
request_moduleI just have to do:modprobe
controller_moduleIf you are sure you need it I'm not against that. But assuming you have udev in your system it should also work like this, without
request_module():1. modprobe
controller_module-> phy device is created in the kernel, uevent sent
2. udev receives uevent, finds matching module and does modprobephy_module
Quoted 7 lines
Remaining part is as before:
phy_moduleregisters the driver which gets matched with phy device;probe()is called which registers v4l2 subdev which then is registered tov4l2_devicethrough the v4l2-async mechanism.All this assumes udev is running and modules are installed in /lib/modules/$(uname -r). E.g. there should be your module alias as shown by modinfo
phy_modulein /lib/modules/$(uname -r)/modules.alias.
The modules are installed but I think I don't have udev :/ I'm running this on an embedded platform called ARC AXS and I'm using buildroot with minimal options.
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +9
-0+ ret =of_platform_populate (dw_dev->of_node,NULL , &lookup, dw_dev->dev); +if (ret) { +dev_err (dw_dev->dev,"failed to populate phy driver\n" ); +return ret; + } + +return 0 ; +} +drivers/media/platform/dwc/dw-hdmi-rx.c
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+static void dw_hdmi_phy_exit (struct dw_hdmi_dev *dw_dev) +{ +of_platform_depopulate (dw_dev->dev); +} +static int dw_hdmi_v4l2_notify_complete (struct v4l2_async_notifier *notifier) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); +int ret; + + ret =v4l2_device_register_subdev_nodes (&dw_dev->v4l2_dev);There shouldn't be multiple struct
v4l2_deviceinstances, instead we should have only one created by the main driver. AFAIU, in your case it would be driver associated with the dw-hdmi-soc DT node. And normally such a top level driver creates subdev device nodes when its all required sub-devices are available.I think this patch could be useful for you:
https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.linuxtv.org_patch_41834&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=uNHRQkbP_-z8v5d30KFx9pcPEUhlr4ciWY3ZDAVExTA&s=dB9wpgeP7AJg1eDRty0-RKhq3DY-7J5srIzyVoJey5I&e=
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
With that the dw-hdmi-soc driver would have it's v4l2-async notifier's
notify_complete()callback called only when both the hdmi-rx and the hdmi-phy subdevs are registered.Yeah, I saw the patches. I just implemented this way because they are not merged yet, right?
I think these patches will be merged in v4.14-rc1, so together with your driver. You could apply them locally and indicate that your series depends on them in the cover letter.
Ok, will apply them locally and re-test.
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 7 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +6
-0+if (ret) { +dev_err (dw_dev->dev,"failed to register subdev nodes\n" ); +return ret; + } + +return 0 ;drivers/media/platform/dwc/dw-hdmi-rx.c line 1748
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+} +static int dw_hdmi_rx_probe (struct platform_device *pdev) +{ +/* V4L2 initialization */ + sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_hdmi_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + sd->internal_ops = &dw_hdmi_internal_ops; + sd->flags |=V4L2_SUBDEV_FL_HAS_EVENTS ;Don't you also need
V4L2_SUBDEV_FL_HAS_DEVNODEflag set?
Quoted 4 lines
Quoted 2 lines
Ouch. Yes I need otherwise the subdev will not be associated with the
v4l2_device.This flag indicates that the v4l2 subdev device node (
/dev/v4l-subdev?) should be created for this subdevice.
Ok, will add for controller driver only then: I think for phy this should not be added because controller is responsible to manage phy entirely so creating a /dev/ which can be seen by userspace can lead to confusion, maybe?
Best regards,
Jose Miguel Abreu
Quoted 5 lines
---
Regards,
Sylwester
Re: [PATCH v4 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver
Hi Jose,
Quoted 10 lines · Jose Abreu
Quoted 8 lines · Sylwester Nawrocki
Quoted 7 lines · Jose Abreu
Quoted 6 lines · Sylwester Nawrocki
Quoted 5 lines · 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.
Quoted 3 lines
The modules are installed but I think I don't have udev :/ I'm running this on an embedded platform called ARC AXS and I'm using buildroot with minimal options.
OK, then let's keep this request_module.
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1561
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +6
-0+static int dw_hdmi_v4l2_notify_complete (struct v4l2_async_notifier *notifier) +{ +struct dw_hdmi_dev *dw_dev =notifier_to_dw_dev (notifier); +int ret; + + ret =v4l2_device_register_subdev_nodes (&dw_dev->v4l2_dev);There shouldn't be multiple struct
v4l2_deviceinstances, instead we should have only one created by the main driver. AFAIU, in your case it would be driver associated with the dw-hdmi-soc DT node. And normally such a top level driver creates subdev device nodes when its all required sub-devices are available.I think this patch could be useful for you:
https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.linuxtv.
org_patch_41834&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19I Jiuxx_p_Rzo2g-uHDKw&m=uNHRQkbP_-z8v5d30KFx9pcPEUhlr4ciWY3ZDAVExTA&s=dB9wpgeP 7AJg1eDRty0-RKhq3DY-7J5srIzyVoJey5I&e=With that the dw-hdmi-soc driver would have it's v4l2-async notifier's
notify_complete()callback called only when both the hdmi-rx and the hdmi-phy subdevs are registered.
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Yeah, I saw the patches. I just implemented this way because they are not merged yet, right?
I think these patches will be merged in v4.14-rc1, so together with your driver. You could apply them locally and indicate that your series depends on them in the cover letter.
Ok, will apply them locally and re-test.
Thanks.
Quoted 21 lines
Quoted 21 lines
Quoted 20 lines
Quoted 10 lines
Quoted 10 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +10
-0+if (ret) { +dev_err (dw_dev->dev,"failed to register subdev nodes\n" ); +return ret; + } + +return 0 ; +} +static int dw_hdmi_rx_probe (struct platform_device *pdev) +{ +/* V4L2 initialization */ Quoted 8 lines
drivers/media/platform/dwc/dw-hdmi-rx.c line 1748
Quoted 6 lines
drivers/media/platform/dwc/dw-hdmi-rx.c new file +6
-0+ sd = &dw_dev->sd; +v4l2_subdev_init (sd, &dw_hdmi_sd_ops); +strlcpy (sd->name,dev_name (dev),sizeof (sd->name)); + sd->dev = dev; + sd->internal_ops = &dw_hdmi_internal_ops; + sd->flags |=V4L2_SUBDEV_FL_HAS_EVENTS ;Don't you also need
V4L2_SUBDEV_FL_HAS_DEVNODEflag set?Ouch. Yes I need otherwise the subdev will not be associated with the
v4l2_device.
Quoted 7 lines
Quoted 2 lines
This flag indicates that the v4l2 subdev device node (
/dev/v4l-subdev?) should be created for this subdevice.Ok, will add for controller driver only then: I think for phy this should not be added because controller is responsible to manage phy entirely so creating a
/dev/which can be seen by userspace can lead to confusion, maybe?
Right, there should be no need for the PHY. It's up to you to set it or not for the RX controller subdev. I just got confused because in your dw_hdmi_sd_ops data structure there are ops which would suggest that device node is used.
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
PATCH v4 3/4 MAINTAINERS: Add entry for Synopsys Designware HDMI drivers
In reply to Jose AbreuAdd a entry for Synopsys Designware HDMI Receivers drivers and phys.
--- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+)
MAINTAINERS +7 -0
@@ -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
PATCH v4 4/4 dt-bindings: media: Document Synopsys Designware HDMI RX
In reply to Jose AbreuDocument the bindings for the Synopsys Designware HDMI RX.
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
@@ -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-clk". + +- 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-clk". + +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; + + dw_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-clk"; + + #address-cells = <1>; + #size-cells = <0>; + + dw_hdmi_phy_e405@fc { + compatible = "snps,dw-hdmi-phy-e405"; + reg = <0xfc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg-clk"; + }; + }; +};
Signature · 1 line
1.9.1
Rob Herring
maintainerRe: [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Quoted 35 lines · 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 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.txtDocumentation/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.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 20
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10
-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. + +- clocks: Phandle to the config clock block. + +- clock-names: Shall be "cfg-clk".
"-clk" is redundant.
Seems strange that this is the only clock. The only other clock is the HDMI clock from the HDMI transmitter.
Quoted 14 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +14
-0+ +- 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.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10
-0+ +- clock-names:Shall be "cfg-clk". + +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";
Not documented.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 50
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6
-0+ reg = <0x11c00 0x1000>; /* EDIDs */ + #address-cells = <1>; + #size-cells = <1>; + ranges; + + dw_hdmi_rx@0 {
hdmi-rx@0
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 54
Quoted 4 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +4
-0+ compatible = "snps,dw-hdmi-rx"; + reg = <0x0 0x10000>; + interrupts = <1 2>; + edid-phandle = <&dw_hdmi_soc>;
Don't need this if it is the parent node.
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 62
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +8
-0+ + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg-clk"; + + #address-cells = <1>; + #size-cells = <0>; + + dw_hdmi_phy_e405@fc {
hdmi-phy@fc
Quoted 12 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +8
-0+ compatible = "snps,dw-hdmi-phy-e405"; + reg = <0xfc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg-clk"; + }; + }; +};Signature · 1 line
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
Re: [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Hi Rob,
Quoted 48 lines · Rob Herring
Quoted 35 lines · 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 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.txtDocumentation/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.Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 20
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +10
-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. + +- clocks: Phandle to the config clock block. + +- clock-names: Shall be "cfg-clk"."-clk" is redundant.
Quoted 2 lines
Seems strange that this is the only clock. The only other clock is the HDMI clock from the HDMI transmitter.
Its a receiver so it gets driven by the transmitter. In my implementation I only need to configure this clock in the controller so that it knows the timebase. I will change to "cfg" only then.
Quoted 18 lines
Quoted 18 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +17
-0+ +- 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-clk". +
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +7
-0+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";Not documented.
Yes, its a sample binding which reflects a wrapper driver that shall instantiate the controller driver (and this wrapper driver is not in this patch series), should I remove this?
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 50
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +6
-0+ reg = <0x11c00 0x1000>; /* EDIDs */ + #address-cells = <1>; + #size-cells = <1>; + ranges; + + dw_hdmi_rx@0 {hdmi-rx@0
Ok.
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 54
Quoted 5 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +4
-0+ compatible = "snps,dw-hdmi-rx"; + reg = <0x0 0x10000>; + interrupts = <1 2>; + edid-phandle = <&dw_hdmi_soc>;Don't need this if it is the parent node.
Sometimes it will not be the parent node (if edid handling is done in a separate driver, for example).
Quoted 10 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 62
Quoted 9 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +8
-0+ + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg-clk"; + + #address-cells = <1>; + #size-cells = <0>; + + dw_hdmi_phy_e405@fc {hdmi-phy@fc
Ok.
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 67
Quoted 6 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +5
-0+ compatible = "snps,dw-hdmi-phy-e405"; + reg = <0xfc>; + + clocks = <&dw_hdmi_refclk>; + clock-names = "cfg-clk";
I will also change this to "cfg" only.
Thanks for the review!
Best regards,
Jose Miguel Abreu
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +3
-0+ }; + }; +};Signature · 1 line
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
Rob Herring
maintainerRe: [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Quoted 7 lines · Jose Abreu
Hi Rob,
Quoted 3 lines · Rob Herring
Quoted 2 lines · Jose Abreu
Document the bindings for the Synopsys Designware HDMI RX.
[...]
Quoted 9 lines
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +7
-0+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";Not documented.
Quoted 3 lines
Yes, its a sample binding which reflects a wrapper driver that shall instantiate the controller driver (and this wrapper driver is not in this patch series), should I remove this?
Ah, I see. Please don't do this wrapper node like what was done on DWC3. It should be all one node with the SoC specific part being a new compatible string (and maybe additional properties). If there's really some custom logic around the IP block, then maybe it makes sense, but if it is just different clock connections, phys, resets, etc. those don't need a separate node.
Rob
--
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
Re: [PATCH v4 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX
Hi Rob,
Quoted 21 lines · Rob Herring
Quoted 7 lines · Jose Abreu
Hi Rob,
Quoted 3 lines · Rob Herring
Quoted 2 lines · Jose Abreu
Document the bindings for the Synopsys Designware HDMI RX.
[...]
Quoted 11 lines
Quoted 8 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt line 44
Quoted 7 lines
Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt new file +7
-0+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";Not documented.
Yes, its a sample binding which reflects a wrapper driver that shall instantiate the controller driver (and this wrapper driver is not in this patch series), should I remove this?
Quoted 6 lines
Ah, I see. Please don't do this wrapper node like what was done on DWC3. It should be all one node with the SoC specific part being a new compatible string (and maybe additional properties). If there's really some custom logic around the IP block, then maybe it makes sense, but if it is just different clock connections, phys, resets, etc. those don't need a separate node.
Ok. I guess I can just drop the SoC specific bindings as this was more of a sample as how the EDID handle can be specified. I just sent v8 now with the new bindings :) Thanks!
Best regards,
Jose Miguel Abreu
Quoted 2 lines
Rob