Re: [PATCH v1 6/6] drm/mediatek: Add mt8195 DisplayPort driver
From: Philipp Zabel <p.zabel@pengutronix.de>
Date: 2021-09-07 08:49:24
Also in:
dri-devel, linux-arm-kernel
Hi Markus, On Mon, 2021-09-06 at 21:35 +0200, Markus Schneider-Pargmann wrote:
This patch adds a DisplayPort driver for the Mediatek mt8195 SoC. It supports both functional units on the mt8195, the embedded DisplayPort as well as the external DisplayPort units. It offers hot-plug-detection, audio up to 8 channels, and DisplayPort 1.4 with up to 4 lanes. This driver is based on an initial version by Jason-JH.Lin [off-list ref]. Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> ---
[...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c new file mode 100644 index 000000000000..1bd07c8d2f69 --- /dev/null +++ b/drivers/gpu/drm/mediatek/mtk_dp.c@@ -0,0 +1,2881 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019 MediaTek Inc. + * Copyright (c) 2021 BayLibre + */ +
[...]
+#include <linux/component.h>
[...]
+#include <linux/extcon.h> +#include <linux/extcon-provider.h>
[...]
+#include <linux/kthread.h> +#include <linux/mfd/syscon.h>
[...]
+#include <linux/of_gpio.h> +#include <linux/of_graph.h>
[...]
+#include <linux/phy/phy.h>
The list of included headers could be pruned a bit, from a quick look it seems like neither of these are actually used. [...]
+static void mtk_dp_ssc_enable(struct mtk_dp *mtk_dp, bool enable)
+{
+ mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE, DP_PWR_STATE_BANDGAP,
+ DP_PWR_STATE_MASK);
+ mtk_dp_update_bits(mtk_dp, DP_PHY_DIG_PLL_CTL_1,
+ enable ? TPLL_SSC_EN : 0, TPLL_SSC_EN);
+ mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE,
+ DP_PWR_STATE_BANDGAP_TPLL_LANE, DP_PWR_STATE_MASK);
+
+ udelay(50);Can usleep_range() be used here? Same for the other delays. [...]
+static void mtk_dp_hpd_sink_event(struct mtk_dp *mtk_dp)
+{[...]
+
+ if (DP_GET_SINK_COUNT(sink_count) &&
+ (link_status[2] & DP_DOWNSTREAM_PORT_STATUS_CHANGED)) {
+ mtk_dp->train_info.check_cap_count = 0;
+ kfree(mtk_dp->edid);
+ mtk_dp->edid = NULL;Should this be protect by a lock? This looks like it could race with the access in mtk_dp_edid_parse_audio_capabilities() or mtk_dp_get_edid() [...]
+static int mtk_dp_train_handler(struct mtk_dp *mtk_dp)
+{
+ int ret = 0;
+
+ ret = mtk_dp_train_hpd_handle(mtk_dp);
+
+ if (!mtk_dp->train_info.cable_plugged_in)
+ return -ENODEV;
+
+ if (mtk_dp->train_state == MTK_DP_TRAIN_STATE_NORMAL)
+ return ret;
+
+ switch (mtk_dp->train_state) {[...]
+ case MTK_DP_TRAIN_STATE_TRAINING:
+ ret = mtk_dp_train_start(mtk_dp);
+ if (!ret) {
+ mtk_dp_video_mute(mtk_dp, true);
+ mtk_dp_audio_mute(mtk_dp, true);
+ mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKTIMING;
+ mtk_dp_fec_enable(mtk_dp, mtk_dp->has_fec);
+ } else if (ret != -EAGAIN)
+ mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;A small whitespace issue and missing braces. Consider running this through checkpatch.pl --strict once for style issues. [...]
+static irqreturn_t mtk_dp_hpd_event(int hpd, void *dev)
+{
+ struct mtk_dp *mtk_dp = dev;
+ uint32_t irq_status;
+
+ irq_status = mtk_dp_read(mtk_dp, MTK_DP_TOP_IRQ_STATUS);
+
+ if (!irq_status)
+ return IRQ_HANDLED;This check seems superfluous given that only the RGS_IRQ_STATUS_TRANSMITTER bit is checked right below:
+ if (irq_status & RGS_IRQ_STATUS_TRANSMITTER) + return mtk_dp_hpd_isr_handler(mtk_dp); + + return IRQ_HANDLED; +}
[...]
+static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
+ struct drm_connector *connector)
+{
+ struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
+ bool pre_enabled = mtk_dp->pre_enabled;
+
+ if (mtk_dp->edid)
+ kfree(mtk_dp->edid);Unnecessary check, kfree() accepts NULL. regards Philipp _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek