| 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: <03a5591a-574f-ec07-68c8-0e3f97a0c68f@collabora.com>
Date: Thu, 24 Feb 2022 14:57:11 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Guillaume Ranquet <granquet@...libre.com>, chunkuang.hu@...nel.org,
p.zabel@...gutronix.de, airlied@...ux.ie, daniel@...ll.ch,
robh+dt@...nel.org, maarten.lankhorst@...ux.intel.com,
mripard@...nel.org, tzimmermann@...e.de, matthias.bgg@...il.com,
chunfeng.yun@...iatek.com, kishon@...com, vkoul@...nel.org,
deller@....de, ck.hu@...iatek.com, jitao.shi@...iatek.com
Cc: dri-devel@...ts.freedesktop.org,
linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-phy@...ts.infradead.org, linux-fbdev@...r.kernel.org,
Markus Schneider-Pargmann <msp@...libre.com>,
kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v8 15/19] drm/mediatek: Add mt8195 Embedded DisplayPort
driver
Il 18/02/22 15:54, Guillaume Ranquet ha scritto:
> From: Markus Schneider-Pargmann <msp@...libre.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@...iatek.com>.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>
> Signed-off-by: Guillaume Ranquet <granquet@...libre.com>
> Reported-by: kernel test robot <lkp@...el.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
Powered by blists - more mailing lists