From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
To: Guillaume Ranquet <granquet@baylibre.com>,
chunkuang.hu@kernel.org, p.zabel@pengutronix.de,
airlied@linux.ie, daniel@ffwll.ch, robh+dt@kernel.org,
maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, matthias.bgg@gmail.com,
chunfeng.yun@mediatek.com, kishon@ti.com, vkoul@kernel.org,
deller@gmx.de, ck.hu@mediatek.com, jitao.shi@mediatek.com
Cc: dri-devel@lists.freedesktop.org,
linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-phy@lists.infradead.org, linux-fbdev@vger.kernel.org,
Markus Schneider-Pargmann <msp@baylibre.com>,
kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v8 15/19] drm/mediatek: Add mt8195 Embedded DisplayPort driver
Date: Thu, 24 Feb 2022 14:57:11 +0100 [thread overview]
Message-ID: <03a5591a-574f-ec07-68c8-0e3f97a0c68f@collabora.com> (raw)
In-Reply-To: <20220218145437.18563-16-granquet@baylibre.com>
Il 18/02/22 15:54, Guillaume Ranquet ha scritto:
> From: Markus Schneider-Pargmann <msp@baylibre.com>
>
> This patch adds a DisplayPort driver for the Mediatek mt8195 SoC.
>
> It supports the mt8195, the embedded DisplayPort units. It offers
> hot-plug-detection and DisplayPort 1.4 with up to 4 lanes.
>
> The driver creates a child device for the phy. The child device will
> never exist without the parent being active. As they are sharing a
> register range, the parent passes a regmap pointer to the child so that
> both can work with the same register range. The phy driver sets device
> data that is read by the parent to get the phy device that can be used
> to control the phy properties.
>
> This driver is based on an initial version by
> Jason-JH.Lin <jason-jh.lin@mediatek.com>.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
> drivers/gpu/drm/mediatek/Kconfig | 7 +
> drivers/gpu/drm/mediatek/Makefile | 2 +
> drivers/gpu/drm/mediatek/mtk_dp.c | 2358 ++++++++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_dp_reg.h | 568 ++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 +
> 6 files changed, 2937 insertions(+)
> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp.c
> create mode 100644 drivers/gpu/drm/mediatek/mtk_dp_reg.h
>
Sorry for the double review, but I've just noticed something critical:
the new version of mtk_dp_train_handler is severely misbehaving, producing
non-functional display.
I also went on with some effort to give you a solution for this,
which implies to also tick the DP_STATE with the DP_TRAIN_STATE;
This is my take on it:
static int mtk_dp_train_handler(struct mtk_dp *mtk_dp)
{
bool training_done = false;
short max_retry = 50;
int ret = 0;
do {
switch (mtk_dp->train_state) {
case MTK_DP_TRAIN_STATE_STARTUP:
mtk_dp_state_handler(mtk_dp);
mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKCAP;
break;
case MTK_DP_TRAIN_STATE_CHECKCAP:
if (mtk_dp_parse_capabilities(mtk_dp)) {
mtk_dp->train_info.check_cap_count = 0;
mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKEDID;
} else {
mtk_dp->train_info.check_cap_count++;
if (mtk_dp->train_info.check_cap_count >
MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT) {
mtk_dp->train_info.check_cap_count = 0;
mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;
ret = -ETIMEDOUT;
}
}
break;
case MTK_DP_TRAIN_STATE_CHECKEDID:
mtk_dp->audio_enable =
!mtk_dp_edid_parse_audio_capabilities(
mtk_dp, &mtk_dp->info.audio_caps);
if (!mtk_dp->audio_enable)
memset(&mtk_dp->info.audio_caps, 0,
sizeof(mtk_dp->info.audio_caps));
mtk_dp->train_state = MTK_DP_TRAIN_STATE_TRAINING_PRE;
break;
case MTK_DP_TRAIN_STATE_TRAINING_PRE:
mtk_dp_state_handler(mtk_dp);
mtk_dp->train_state = MTK_DP_TRAIN_STATE_TRAINING;
break;
case MTK_DP_TRAIN_STATE_TRAINING:
ret = mtk_dp_train_start(mtk_dp);
if (ret == 0) {
mtk_dp_video_mute(mtk_dp, true);
mtk_dp_audio_mute(mtk_dp, true);
mtk_dp->train_state = MTK_DP_TRAIN_STATE_NORMAL;
mtk_dp_fec_enable(mtk_dp, mtk_dp->has_fec);
} else if (ret != -EAGAIN) {
mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;
}
break;
case MTK_DP_TRAIN_STATE_NORMAL:
mtk_dp_state_handler(mtk_dp);
training_done = true;
break;
case MTK_DP_TRAIN_STATE_DPIDLE:
break;
default:
break;
}
if (ret) {
if (ret == -EAGAIN)
continue;
/*
* If we get any other error number, it doesn't
* make any sense to keep iterating.
*/
break;
}
} while (!training_done || --max_retry);
return ret;
}
> +static void mtk_dp_train_handler(struct mtk_dp *mtk_dp)
> +{
> + int ret = 0;
> + int i = 50;
> +
> + do {
> + if (mtk_dp->train_state == MTK_DP_TRAIN_STATE_NORMAL)
> + continue;
> +
> + switch (mtk_dp->train_state) {
> + case MTK_DP_TRAIN_STATE_STARTUP:
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKCAP;
> + break;
> +
> + case MTK_DP_TRAIN_STATE_CHECKCAP:
> + if (mtk_dp_parse_capabilities(mtk_dp)) {
> + mtk_dp->train_info.check_cap_count = 0;
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_CHECKEDID;
> + } else {
> + mtk_dp->train_info.check_cap_count++;
> +
> + if (mtk_dp->train_info.check_cap_count >
> + MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT) {
> + mtk_dp->train_info.check_cap_count = 0;
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;
> + ret = -ETIMEDOUT;
> + }
> + }
> + break;
> +
> + case MTK_DP_TRAIN_STATE_CHECKEDID:
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_TRAINING_PRE;
> + break;
> +
> + case MTK_DP_TRAIN_STATE_TRAINING_PRE:
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_TRAINING;
> + break;
> +
> + case MTK_DP_TRAIN_STATE_TRAINING:
> + ret = mtk_dp_train_start(mtk_dp);
> + if (!ret) {
> + mtk_dp_video_mute(mtk_dp, true);
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_NORMAL;
> + mtk_dp_fec_enable(mtk_dp, mtk_dp->has_fec);
> + } else if (ret != -EAGAIN) {
> + mtk_dp->train_state = MTK_DP_TRAIN_STATE_DPIDLE;
> + }
> +
> + ret = 0;
> + break;
> +
> + case MTK_DP_TRAIN_STATE_NORMAL:
> + break;
> + case MTK_DP_TRAIN_STATE_DPIDLE:
> + break;
> + default:
> + break;
> + }
> + } while (ret && i--);
> +
> + if (ret)
> + drm_err(mtk_dp->drm_dev, "Train handler failed %d\n", ret);
> +}
> +
> +static void mtk_dp_video_enable(struct mtk_dp *mtk_dp, bool enable)
> +{
> + if (enable) {
> + mtk_dp_set_tx_out(mtk_dp);
> + mtk_dp_video_mute(mtk_dp, false);
> + } else {
> + mtk_dp_video_mute(mtk_dp, true);
> + }
> +}
> +
> +static void mtk_dp_video_config(struct mtk_dp *mtk_dp)
> +{
> + mtk_dp_mn_overwrite_disable(mtk_dp);
> +
> + mtk_dp_set_msa(mtk_dp);
> +
> + mtk_dp_set_color_depth(mtk_dp, mtk_dp->info.depth);
> + mtk_dp_set_color_format(mtk_dp, mtk_dp->info.format);
> +}
> +
> +static void mtk_dp_state_handler(struct mtk_dp *mtk_dp)
> +{
> + switch (mtk_dp->state) {
> + case MTK_DP_STATE_INITIAL:
> + mtk_dp_video_mute(mtk_dp, true);
> + mtk_dp->state = MTK_DP_STATE_IDLE;
> + break;
> +
> + case MTK_DP_STATE_IDLE:
> + if (mtk_dp->train_state == MTK_DP_TRAIN_STATE_NORMAL)
> + mtk_dp->state = MTK_DP_STATE_PREPARE;
> + break;
> +
> + case MTK_DP_STATE_PREPARE:
> + mtk_dp_video_config(mtk_dp);
> + mtk_dp_video_enable(mtk_dp, true);
> +
> + mtk_dp->state = MTK_DP_STATE_NORMAL;
> + break;
> +
> + case MTK_DP_STATE_NORMAL:
> + if (mtk_dp->train_state != MTK_DP_TRAIN_STATE_NORMAL) {
> + mtk_dp_video_mute(mtk_dp, true);
> + mtk_dp->state = MTK_DP_STATE_IDLE;
> + }
> + break;
> +
> + default:
> + break;
> + }
> +}
> +
Move mtk_dp_train_handler() here.....
> +static void mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> + struct drm_bridge_state *old_state)
> +{
> + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> + struct drm_connector_state *conn_state;
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
int ret;
> +
> + mtk_dp->conn = drm_atomic_get_new_connector_for_encoder(old_state->base.state,
> + bridge->encoder);
> + if (!mtk_dp->conn) {
> + drm_err(mtk_dp->drm_dev,
> + "Can't enable bridge as connector is missing\n");
> + return;
> + }
> +
> + memcpy(mtk_dp->connector_eld, mtk_dp->conn->eld, MAX_ELD_BYTES);
> +
> + conn_state =
> + drm_atomic_get_new_connector_state(old_state->base.state, mtk_dp->conn);
> + if (!conn_state) {
> + drm_err(mtk_dp->drm_dev,
> + "Can't enable bridge as connector state is missing\n");
> + return;
> + }
> +
> + crtc = conn_state->crtc;
> + if (!crtc) {
> + drm_err(mtk_dp->drm_dev,
> + "Can't enable bridge as connector state doesn't have a crtc\n");
> + return;
> + }
> +
> + crtc_state = drm_atomic_get_new_crtc_state(old_state->base.state, crtc);
> + if (!crtc_state) {
> + drm_err(mtk_dp->drm_dev,
> + "Can't enable bridge as crtc state is missing\n");
> + return;
> + }
> +
> + mtk_dp_parse_drm_mode_timings(mtk_dp, &crtc_state->adjusted_mode);
> + if (!mtk_dp_parse_capabilities(mtk_dp)) {
> + drm_err(mtk_dp->drm_dev,
> + "Can't enable bridge as nothing is plugged in\n");
> + return;
> + }
> +
> + /* Training */
The following is very similar to your previous version of this function:
ret = mtk_dp_train_handler(mtk_dp);
if (ret) {
drm_err(mtk_dp->drm_dev, "Train handler failed %d\n", ret);
return;
}
mtk_dp->enabled = true;
mtk_dp_update_plugged_status(mtk_dp)
}
> + mtk_dp_train_handler(mtk_dp);
> + mtk_dp_state_handler(mtk_dp);
> + mtk_dp->enabled = true;
> +}
Regards,
Angelo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-02-24 13:58 UTC|newest]
Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-18 14:54 [PATCH v8 0/19] drm/mediatek: Add mt8195 DisplayPort driver Guillaume Ranquet
2022-02-18 14:54 ` [PATCH v8 01/19] dt-bindings: mediatek,dpi: Add DP_INTF compatible Guillaume Ranquet
2022-02-18 14:54 ` [PATCH v8 02/19] dt-bindings: mediatek,dp: Add Display Port binding Guillaume Ranquet
2022-02-21 2:36 ` [PATCH v8 02/19] dt-bindings: mediatek, dp: " Rob Herring
2022-02-18 14:54 ` [PATCH v8 03/19] drm/edid: Add cea_sad helpers for freq/length Guillaume Ranquet
2022-02-21 14:30 ` AngeloGioacchino Del Regno
2022-02-25 10:35 ` Guillaume Ranquet
2022-02-18 14:54 ` [PATCH v8 04/19] video/hdmi: Add audio_infoframe packing for DP Guillaume Ranquet
2022-02-21 14:30 ` AngeloGioacchino Del Regno
2022-02-25 10:38 ` Guillaume Ranquet
2022-02-22 15:16 ` AngeloGioacchino Del Regno
2022-02-25 10:40 ` Guillaume Ranquet
2022-02-18 14:54 ` [PATCH v8 05/19] drm/mediatek: dpi: move dpi limits to board config Guillaume Ranquet
2022-02-21 1:46 ` Chun-Kuang Hu
2022-02-25 10:45 ` Guillaume Ranquet
2022-02-21 14:30 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 06/19] drm/mediatek: dpi: implement a CK/DE pol toggle in " Guillaume Ranquet
2022-02-21 2:14 ` Chun-Kuang Hu
2022-02-25 10:52 ` Guillaume Ranquet
2022-02-21 14:30 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 07/19] drm/mediatek: dpi: implement a swap_input " Guillaume Ranquet
2022-02-21 2:32 ` Chun-Kuang Hu
2022-02-25 10:55 ` Guillaume Ranquet
2022-02-18 14:54 ` [PATCH v8 08/19] drm/mediatek: dpi: move dimension mask to " Guillaume Ranquet
2022-02-21 2:40 ` Chun-Kuang Hu
2022-02-21 2:42 ` Chun-Kuang Hu
2022-02-21 14:30 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 09/19] drm/mediatek: dpi: move dimension_mask " Guillaume Ranquet
2022-02-21 2:43 ` Chun-Kuang Hu
2022-02-21 14:31 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 10/19] drm/mediatek: dpi: move swap_shift " Guillaume Ranquet
2022-02-21 3:24 ` Chun-Kuang Hu
2022-02-25 11:10 ` Guillaume Ranquet
2022-02-21 14:31 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 11/19] drm/mediatek: dpi: move the yuv422_en_bit " Guillaume Ranquet
2022-02-21 3:31 ` Chun-Kuang Hu
2022-02-21 14:31 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 12/19] drm/mediatek: dpi: move the csc_enable bit " Guillaume Ranquet
2022-02-21 3:33 ` Chun-Kuang Hu
2022-02-21 14:31 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 13/19] drm/mediatek: dpi: Add dpintf support Guillaume Ranquet
2022-02-21 4:34 ` Chun-Kuang Hu
2022-02-21 9:44 ` Maxime Ripard
2022-02-25 11:25 ` Guillaume Ranquet
2022-02-21 14:30 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 14/19] phy: phy-mtk-dp: Add driver for DP phy Guillaume Ranquet
2022-02-21 14:31 ` AngeloGioacchino Del Regno
2022-02-25 11:49 ` Guillaume Ranquet
2022-02-25 12:06 ` AngeloGioacchino Del Regno
2022-02-18 14:54 ` [PATCH v8 15/19] drm/mediatek: Add mt8195 Embedded DisplayPort driver Guillaume Ranquet
2022-02-21 14:31 ` AngeloGioacchino Del Regno
2022-02-24 13:57 ` AngeloGioacchino Del Regno [this message]
2022-02-25 9:45 ` CK Hu
2022-02-25 13:04 ` Guillaume Ranquet
2022-03-04 2:42 ` CK Hu
2022-02-18 14:54 ` [PATCH v8 16/19] drm/mediatek: Add mt8195 External DisplayPort support Guillaume Ranquet
2022-02-18 14:54 ` [PATCH v8 17/19] drm/mediatek: add hpd debounce Guillaume Ranquet
2022-03-16 6:48 ` Rex-BC Chen
2022-03-18 9:03 ` Wei-Shun Chang
2022-03-21 7:24 ` Rex-BC Chen
2022-02-18 14:54 ` [PATCH v8 18/19] drm/mediatek: change the aux retries times when receiving AUX_DEFER Guillaume Ranquet
2022-03-16 13:35 ` Chen-Yu Tsai
2022-02-18 14:54 ` [PATCH v8 19/19] drm/mediatek: DP audio support for mt8195 Guillaume Ranquet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=03a5591a-574f-ec07-68c8-0e3f97a0c68f@collabora.com \
--to=angelogioacchino.delregno@collabora.com \
--cc=airlied@linux.ie \
--cc=chunfeng.yun@mediatek.com \
--cc=chunkuang.hu@kernel.org \
--cc=ck.hu@mediatek.com \
--cc=daniel@ffwll.ch \
--cc=deller@gmx.de \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=granquet@baylibre.com \
--cc=jitao.shi@mediatek.com \
--cc=kishon@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-phy@lists.infradead.org \
--cc=lkp@intel.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthias.bgg@gmail.com \
--cc=mripard@kernel.org \
--cc=msp@baylibre.com \
--cc=p.zabel@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=tzimmermann@suse.de \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).