On Wed, 2022-06-08 at 10:23 +0800, CK Hu wrote:
Hi, Rex:
On Tue, 2022-06-07 at 20:24 +0800, Rex-BC Chen wrote:
On Tue, 2022-06-07 at 14:21 +0800, CK Hu wrote:
Hi, Rex:
On Mon, 2022-05-23 at 12:47 +0200, Guillaume Ranquet wrote:
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 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
[snip]
+static irqreturn_t mtk_dp_hpd_event_thread(int hpd, void *dev) +{
- struct mtk_dp *mtk_dp = dev;
- int event;
- u8 buf[DP_RECEIVER_CAP_SIZE] = {};
- event = mtk_dp_plug_state(mtk_dp) ?
connector_status_connected :
connector_status_disc onnected;
- if (event < 0)
event is always > 0, isn't it?
Hello CK,
ok, I will move this to dp patch.
return IRQ_HANDLED;- if (mtk_dp->drm_dev) {
dev_info(mtk_dp->dev,"drm_helper_hpd_irq_event\n");
drm_helper_hpd_irq_event(mtk_dp->bridge.dev);I think this ISR would come once. If bridge has not attached, the drm core would lost this event. Maybe you should enable eDP hardware after bridge attached or send this event when attached.
for edp patch, I will move it to (mtk_dp_bridge_attach). for dp patch, I will add it back.
I find out that mtk_dp_poweron() is in top of mtk_dp_bridge_attach(). If move mtk_dp_poweron() to bottom of mtk_dp_bridge_attach(), mtk_dp-
drm_dev would not be NULL here. So we could drop this checking.
Hello CK,
If we failed to setup phy(ret!=0), we alos need to deattach this bridge. I don't think it's a good idea just for remove this.
- }
- if (mtk_dp->train_info.cable_state_change) {
Executing this thread imply cable_state_change = true, so drop cable_state_change.
In mtk_dp_hpd_isr_handler(), there is another irq "MTK_DP_HPD_INTERRUPT" which means the sink devices give a interrupt to source device. it's not about connected status, so I think we still need this.
In bottom of mtk_dp_hpd_isr_handler(), the code is:
- train_info->cable_state_change = true;
- return IRQ_WAKE_THREAD;
This thread is called only when return IRQ_WAKE_THREAD, and before return IRQ_WAKE_THREAD, train_info->cable_state_change is always set to true. So in this thread, train_info->cable_state_change must be true.
As mentioned, this irq handler function is not only for connected status.
this could be return if this irq is interrupt from sink device. + if (!(train_info->irq_status & + (MTK_DP_HPD_CONNECT | MTK_DP_HPD_DISCONNECT))) + return IRQ_HANDLED;
BRs, Bo-Chen
Regards, CK
mtk_dp->train_info.cable_state_change = false;mtk_dp->train_state =MTK_DP_TRAIN_STATE_STARTUP;
if (!mtk_dp->train_info.cable_plugged_in ||!mtk_dp_plug_state(mtk_dp)) {I do not like two variable to present one thing. If
mtk_dp->train_info.cable_plugged_in = false and mtk_dp_plug_state(mtk_dp) = ture
What does this mean? I think this mean 'now' is connected because cable_plugged_in is old information and mtk_dp_plug_state() is current information.
But I would like to keep cable_plugged_in and drop mtk_dp_plug_state() because cable_plugged_in would be changed in isr and it would be the same as mtk_dp_plug_state().
Regards, CK
ok, I will drop this.
BRs, Rex
mtk_dp_video_mute(mtk_dp, true);mtk_dp_initialize_priv_data(mtk_dp);mtk_dp_set_idle_pattern(mtk_dp, true);if (mtk_dp->has_fec)mtk_dp_fec_enable(mtk_dp,false);
mtk_dp_update_bits(mtk_dp,MTK_DP_TOP_PWR_STATE,
DP_PWR_STATE_BANDGAP_TPLL,
DP_PWR_STATE_MASK);} else {mtk_dp_update_bits(mtk_dp,MTK_DP_TOP_PWR_STATE,
DP_PWR_STATE_BANDGAP_TPLL_LA NE,
DP_PWR_STATE_MASK);drm_dp_read_dpcd_caps(&mtk_dp->aux,buf);
mtk_dp->train_info.link_rate =min_t(int, mtk_dp-max_linkrate,
buf[mtk_dp-max_linkrate]);
mtk_dp->train_info.lane_count =min_t(int, mtk_dp->max_lanes,drm_dp_max_lane_count(buf));
}- }
- if (mtk_dp->train_info.irq_status &
MTK_DP_HPD_INTERRUPT) {
dev_dbg(mtk_dp->dev, "MTK_DP_HPD_INTERRUPT\n");mtk_dp->train_info.irq_status &=~MTK_DP_HPD_INTERRUPT;
mtk_dp_hpd_sink_event(mtk_dp);- }
- return IRQ_HANDLED;
+}