Re: [PATCH v6 11/11] drm/mediatek: dp: Don't register HPD interrupt handler for eDP case

[AngeloGioacchino Del Regno]

Il 25/07/23 08:37, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Mon, 2023-07-17 at 16:14 +0200, AngeloGioacchino Del Regno wrote:
> >   	
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >   The interrupt handler for HPD is useful only if a display is
> > actually
> > supposed to be hotpluggable, as that manages the machinery to perform
> > cable (un)plug detection, debouncing and setup for re-training.
> >
> > Since eDP panels are not supposed to be hotpluggable we can avoid
> > using the HPD interrupts altogether and rely on HPD polling only
> > for the suspend/resume case, saving us some spinlocking action and
> > the overhead of interrupts firing at every suspend/resume cycle,
> > achieving a faster (even if just slightly) display resume.
> >
> > Signed-off-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@xxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_dp.c | 81 ++++++++++++++++++-----------
> > --
> >   1 file changed, 46 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> > b/drivers/gpu/drm/mediatek/mtk_dp.c
> > index e74295ba9707..c06fcc7318e7 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> > @@ -2182,9 +2182,11 @@ static int mtk_dp_bridge_attach(struct
> > drm_bridge *bridge,
> >   
> >   	mtk_dp->drm_dev = bridge->dev;
> >   
> > -	irq_clear_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
> > -	enable_irq(mtk_dp->irq);
> > -	mtk_dp_hwirq_enable(mtk_dp, true);
> > +	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
> > +		irq_clear_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
> > +		enable_irq(mtk_dp->irq);
> > +		mtk_dp_hwirq_enable(mtk_dp, true);
> > +	}
> >   
> >   	return 0;
> >   
> > @@ -2199,8 +2201,10 @@ static void mtk_dp_bridge_detach(struct
> > drm_bridge *bridge)
> >   {
> >   	struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> >   
> > -	mtk_dp_hwirq_enable(mtk_dp, false);
> > -	disable_irq(mtk_dp->irq);
> > +	if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP) {
> > +		mtk_dp_hwirq_enable(mtk_dp, false);
> > +		disable_irq(mtk_dp->irq);
> > +	}
> >   	mtk_dp->drm_dev = NULL;
> >   	mtk_dp_poweroff(mtk_dp);
> >   	drm_dp_aux_unregister(&mtk_dp->aux);
> > @@ -2579,40 +2583,44 @@ static int mtk_dp_probe(struct
> > platform_device *pdev)
> >   	mtk_dp->dev = dev;
> >   	mtk_dp->data = (struct mtk_dp_data
> > *)of_device_get_match_data(dev);
> >   
> > -	mtk_dp->irq = platform_get_irq(pdev, 0);
> > -	if (mtk_dp->irq < 0)
> > -		return dev_err_probe(dev, mtk_dp->irq,
> > -				     "failed to request dp irq
> > resource\n");
> > -
> > -	mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node,
> > 1, 0);
> > -	if (IS_ERR(mtk_dp->next_bridge) &&
> > -	    PTR_ERR(mtk_dp->next_bridge) == -ENODEV)
> > -		mtk_dp->next_bridge = NULL;
> > -	else if (IS_ERR(mtk_dp->next_bridge))
> > -		return dev_err_probe(dev, PTR_ERR(mtk_dp->next_bridge),
> > -				     "Failed to get bridge\n");
> > -
>
> Why remove next_bridge setting?

Hello CK,

Thanks for making me notice that! The removal of this code snippet belongs
to patch [9/11] `drm/mediatek: dp: Add support for embedded DisplayPort aux-bus`
because in that patch I am moving the call to devm_drm_of_get_bridge() to function
mtk_dp_edp_link_panel(), which is called as a .done_probing() callback if the eDP
panel is on aux-bus, or "manually" if not on aux-bus.

I'll send a v7 with the removal of this snippet in the right patch.

Thanks again,
Angelo