Re: [PATCH v4 4/4] media: platform: synopsys: Add support for hdmi input driver
flat view
From: George Stark <hidden>
Date: 2024-07-20 23:43:53
Also in:
linux-devicetree, linux-media, linux-rockchip, lkml
Hello everybody On 7/19/24 15:40, Shreeya Patel wrote:
Add initial support for the Synopsys DesignWare HDMI RX
Controller Driver used by Rockchip RK3588. The driver
supports:
- HDMI 1.4b and 2.0 modes (HDMI 4k@60Hz)
- RGB888, YUV422, YUV444 and YCC420 pixel formats
- CEC
- EDID configuration
The hardware also has Audio and HDCP capabilities, but these are
not yet supported by the driver.
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Co-developed-by: Dingxian Wen <redacted>
Signed-off-by: Dingxian Wen <redacted>
Signed-off-by: Shreeya Patel <redacted>
---
Changes in v4 :-
- Create a separate config option for selecting the EDID
and enable it by default
- Improve the comment related to DV timings and move it
to the side of hdmirx_get_detected_timings
- Add 100ms delay before pulling the HPD high
- Do not return the detected timings from VIDIOC_G_DV_TIMINGS
- Drop the bus info from hdmirx_querycap
- If *num_planes != 0 then return 0 in hdmirx_queue_setup
- Set queue->min_queued_buffers to 1
- Drop q->allow_cache_hints = 0; as it's always 0 by default
- Add a comment for q->dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS;
- Drop .read = vb2_fop_read as it's not supported by driver
- Remove redundant edid_init_data_600M
- Make HPD low when driver is loaded
- Add support for reading AVI Infoframe
- Remove msg_len checks from hdmirx_cec_transmit
- Add info about the CEC compliance test in the cover letter
- Add arbitration lost status
- Validate the physical address inside the EDID
Changes in v3 :-
- Use v4l2-common helper functions
Changes in v2 :-
- Fix checkpatch --strict warnings
- Rename resets, vo1-grf and HPD node names as per the DT changes
drivers/media/platform/Kconfig | 1 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/synopsys/Kconfig | 3 +
drivers/media/platform/synopsys/Makefile | 2 +
.../media/platform/synopsys/hdmirx/Kconfig | 27 +
.../media/platform/synopsys/hdmirx/Makefile | 4 +
.../platform/synopsys/hdmirx/snps_hdmirx.c | 2763 +++++++++++++++++
.../platform/synopsys/hdmirx/snps_hdmirx.h | 394 +++
.../synopsys/hdmirx/snps_hdmirx_cec.c | 285 ++
.../synopsys/hdmirx/snps_hdmirx_cec.h | 44 +
10 files changed, 3524 insertions(+)
create mode 100644 drivers/media/platform/synopsys/Kconfig
create mode 100644 drivers/media/platform/synopsys/Makefile
create mode 100644 drivers/media/platform/synopsys/hdmirx/Kconfig
create mode 100644 drivers/media/platform/synopsys/hdmirx/Makefile
create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx_cec.c
create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx_cec.h...
quoted hunk
diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c new file mode 100644 index 000000000000..1dfecf256393 --- /dev/null +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c@@ -0,0 +1,2763 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
...
quoted hunk
+} + +static int hdmirx_probe(struct platform_device *pdev) +{ + struct snps_hdmirx_dev *hdmirx_dev; + struct device *dev = &pdev->dev; + struct v4l2_ctrl_handler *hdl; + struct hdmirx_stream *stream; + struct v4l2_device *v4l2_dev; + int ret; + + hdmirx_dev = devm_kzalloc(dev, sizeof(*hdmirx_dev), GFP_KERNEL); + if (!hdmirx_dev) + return -ENOMEM; + + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32)); + if (ret) + return ret; + + hdmirx_dev->dev = dev; + dev_set_drvdata(dev, hdmirx_dev); + + ret = hdmirx_parse_dt(hdmirx_dev); + if (ret) + return ret; + + ret = hdmirx_setup_irq(hdmirx_dev, pdev); + if (ret) + return ret; + + hdmirx_dev->regs = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(hdmirx_dev->regs)) + return dev_err_probe(dev, PTR_ERR(hdmirx_dev->regs), + "failed to remap regs resource\n"); + + mutex_init(&hdmirx_dev->stream_lock); + mutex_init(&hdmirx_dev->work_lock);
AFAIK there's no unified opinion on droping mutex_destroy. Is it so accepted in media subsystem or should we used devm_mutex_init?
quoted hunk
+ spin_lock_init(&hdmirx_dev->rst_lock); + + init_completion(&hdmirx_dev->cr_write_done); + init_completion(&hdmirx_dev->timer_base_lock); + init_completion(&hdmirx_dev->avi_pkt_rcv); + + INIT_WORK(&hdmirx_dev->work_wdt_config, hdmirx_work_wdt_config); + INIT_DELAYED_WORK(&hdmirx_dev->delayed_work_hotplug, + hdmirx_delayed_work_hotplug); + INIT_DELAYED_WORK(&hdmirx_dev->delayed_work_res_change, + hdmirx_delayed_work_res_change); + INIT_DELAYED_WORK(&hdmirx_dev->delayed_work_heartbeat, + hdmirx_delayed_work_heartbeat); + + hdmirx_dev->cur_fmt_fourcc = V4L2_PIX_FMT_BGR24; + hdmirx_dev->timings = cea640x480; + + hdmirx_enable(dev); + hdmirx_init(hdmirx_dev); + + v4l2_dev = &hdmirx_dev->v4l2_dev; + strscpy(v4l2_dev->name, dev_name(dev), sizeof(v4l2_dev->name)); + + hdl = &hdmirx_dev->hdl; + v4l2_ctrl_handler_init(hdl, 1); + + hdmirx_dev->detect_tx_5v_ctrl = v4l2_ctrl_new_std(hdl, NULL, + V4L2_CID_DV_RX_POWER_PRESENT, + 0, 1, 0, 0); + + hdmirx_dev->rgb_range = v4l2_ctrl_new_std_menu(hdl, 0, + V4L2_CID_DV_RX_RGB_RANGE, + V4L2_DV_RGB_RANGE_FULL, 0, + V4L2_DV_RGB_RANGE_AUTO); + + hdmirx_dev->rgb_range->flags |= V4L2_CTRL_FLAG_READ_ONLY; + + if (hdl->error) { + dev_err(dev, "v4l2 ctrl handler init failed\n"); + ret = hdl->error; + goto err_pm; + } + hdmirx_dev->v4l2_dev.ctrl_handler = hdl; + + ret = v4l2_device_register(dev, &hdmirx_dev->v4l2_dev); + if (ret < 0) { + dev_err(dev, "register v4l2 device failed\n"); + goto err_hdl; + } + + stream = &hdmirx_dev->stream; + stream->hdmirx_dev = hdmirx_dev; + ret = hdmirx_register_stream_vdev(stream); + if (ret < 0) { + dev_err(dev, "register video device failed\n"); + goto err_unreg_v4l2_dev; + } + + ret = hdmirx_register_cec(hdmirx_dev, pdev); + if (ret) + goto err_unreg_video_dev; + + hdmirx_load_default_edid(hdmirx_dev); + + hdmirx_enable_irq(dev); + + return 0; + +err_unreg_video_dev: + video_unregister_device(&hdmirx_dev->stream.vdev); +err_unreg_v4l2_dev: + v4l2_device_unregister(&hdmirx_dev->v4l2_dev); +err_hdl: + v4l2_ctrl_handler_free(&hdmirx_dev->hdl); +err_pm: + hdmirx_disable(dev); + + return ret; +}
... -- Best regards George