| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0d67019e-dac5-489c-a42a-dab5028c1bce@salutedevices.com>
Date: Sun, 21 Jul 2024 02:43:44 +0300
From: George Stark <gnstark@...utedevices.com>
To: Shreeya Patel <shreeya.patel@...labora.com>, <heiko@...ech.de>,
<mchehab@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <mturquette@...libre.com>, <sboyd@...nel.org>,
<p.zabel@...gutronix.de>, <jose.abreu@...opsys.com>,
<nelson.costa@...opsys.com>, <shawn.wen@...k-chips.com>,
<nicolas.dufresne@...labora.com>, <hverkuil@...all.nl>,
<hverkuil-cisco@...all.nl>
CC: <kernel@...labora.com>, <linux-kernel@...r.kernel.org>,
<linux-media@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-rockchip@...ts.infradead.org>,
Dmitry Osipenko <dmitry.osipenko@...labora.com>
Subject: Re: [PATCH v4 4/4] media: platform: synopsys: Add support for hdmi
input driver
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@...z)
> - RGB888, YUV422, YUV444 and YCC420 pixel formats
> - CEC
> - EDID configuration
>
> The hardware also has Audio and HDCP capabilities, but these are
> not yet supported by the driver.
>
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>
> Tested-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>
> Co-developed-by: Dingxian Wen <shawn.wen@...k-chips.com>
> Signed-off-by: Dingxian Wen <shawn.wen@...k-chips.com>
> Signed-off-by: Shreeya Patel <shreeya.patel@...labora.com>
> ---
>
> 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
Powered by blists - more mailing lists