On Mon, 2022-07-25 at 17:23 +0800, CK Hu wrote: > Hi, Bo-Chen: > > On Tue, 2022-07-12 at 19:12 +0800, Bo-Chen Chen wrote: > > From: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxx> > > > > Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx> > > Signed-off-by: Guillaume Ranquet <granquet@xxxxxxxxxxxx> > > Signed-off-by: Bo-Chen Chen <rex-bc.chen@xxxxxxxxxxxx> > > --- > > [snip] > > > + > > +/* > > + * We need to handle HPD signal in eDP even though eDP is a always > > connected > > + * device. Besides connected status, there is another feature for > > HPD signal - > > + * HPD pulse: it presents an IRQ from sink devices to source > > devices > > (Refer to > > + * 5.1.4 of DP1.4 spec). > > + */ > > +static irqreturn_t mtk_dp_hpd_isr_handler(struct mtk_dp *mtk_dp) > > +{ > > + bool hpd_change = false; > > + u32 irq_status = mtk_dp_swirq_get_clear(mtk_dp) | > > + mtk_dp_hwirq_get_clear(mtk_dp); > > + struct mtk_dp_train_info *train_info = &mtk_dp->train_info; > > + > > + if (!irq_status) > > + return IRQ_HANDLED; > > + > > + if (irq_status & MTK_DP_HPD_INTERRUPT) > > + train_info->irq_sta.hpd_inerrupt = true; > > + if (irq_status & MTK_DP_HPD_CONNECT || > > + irq_status & MTK_DP_HPD_DISCONNECT) > > + hpd_change = true; > > + > > + if (!(hpd_change)) > > + return IRQ_WAKE_THREAD; > > + > > + if (mtk_dp_plug_state(mtk_dp)) > > mtk_dp_plug_state() is called only here, and prevent function call in > isr handler, so squash mtk_dp_plug_state() into this function. > Hello CK, Thanks for review. I would like to keep this because we will use this function for mtk_dp_plug_state_avoid_pulse() in dp patch. > > + train_info->cable_plugged_in = true; > > + else > > + train_info->cable_plugged_in = false; > > + > > + train_info->cable_state_change = true; > > + > > + return IRQ_WAKE_THREAD; > > +} > > + > > +static irqreturn_t mtk_dp_hpd_event(int hpd, void *dev) > > +{ > > + struct mtk_dp *mtk_dp = dev; > > + u32 irq_status; > > + > > + irq_status = mtk_dp_read(mtk_dp, MTK_DP_TOP_IRQ_STATUS); > > + > > + if (!irq_status) > > + return IRQ_HANDLED; > > + > > + if (irq_status & RGS_IRQ_STATUS_TRANSMITTER) > > + return mtk_dp_hpd_isr_handler(mtk_dp); > > Prevent function call in isr handler, squash mtk_dp_hpd_isr_handler() > into this function. > Is this really necessary? We also modify this function in following patches. I think it's not a good idea to expand this. BRs, Bo-Chen > Regards, > CK > > > + > > + return IRQ_HANDLED; > > +} > > + > >