Re: [PATCH v4 4/4] media: platform: synopsys: Add support for hdmi input driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>
Tested-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
Co-developed-by: Dingxian Wen <shawn.wen@xxxxxxxxxxxxxx>
Signed-off-by: Dingxian Wen <shawn.wen@xxxxxxxxxxxxxx>
Signed-off-by: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx>
---

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

...
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
+/*

...

+}
+
+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?


> +	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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux