Re: [PATCH v16 3/8] drm/mediatek: Add MT8195 Embedded DisplayPort driver

[Bo-Chen Chen <rex-bc.chen@mediatek.com>]

On Mon, 2022-08-08 at 13:21 +0800, CK Hu wrote:
> Hi, Bo-Chen:
> 
> On Fri, 2022-08-05 at 18:14 +0800, Bo-Chen Chen wrote:
> > From: Markus Schneider-Pargmann <msp@baylibre.com>
> > 
> > This patch adds a embedded displayport driver for the MediaTek
> > mt8195
> > SoC.
> > 
> > It supports the MT8195, the embedded DisplayPort units. It offers
> > 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
> > Jitao shi <jitao.shi@mediatek.com>
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> > Signed-off-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
> > Tested-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> 
> [snip]
> 
> > +
> > +static void mtk_dp_hpd_sink_event(struct mtk_dp *mtk_dp)
> > +{
> > +	ssize_t ret;
> > +	u8 link_status[DP_LINK_STATUS_SIZE] = {};
> > +	u32 link_status_reg = DP_LANE0_1_STATUS;
> > +
> > +	ret = drm_dp_dpcd_read(&mtk_dp->aux, link_status_reg,
> > link_status,
> > +			       sizeof(link_status));
> > +	if (!ret) {
> > +		drm_err(mtk_dp->drm_dev, "Read link status failed\n");
> > +		return;
> > +	}
> > +
> > +	if (!drm_dp_channel_eq_ok(link_status, mtk_dp-
> > > train_info.lane_count)) {
> > 
> > +		drm_err(mtk_dp->drm_dev, "Channel EQ failed\n");
> > +		return;
> > +	}
> > +
> > +	if (link_status[1] & DP_REMOTE_CONTROL_COMMAND_PENDING)
> 
> I does not see any other DP driver process
> DP_REMOTE_CONTROL_COMMAND_PENDING, why mtk dp driver process it? If
> this is an advanced function, separate this to an independent patch.
> 
> Regards,
> CK
> 

Hello CK,

We are not using this. The dpcd write is just for clearing the irq
status of sink device, but we are not doing anything for this hpd
event. So I will drop this.

BRs,
Bo-Chen

> > +		drm_dp_dpcd_writeb(&mtk_dp->aux,
> > DP_DEVICE_SERVICE_IRQ_VECTOR,
> > +				   DP_REMOTE_CONTROL_COMMAND_PENDING);
> > +}
> > +
> > 
> 
>