Re: [PATCH v7 06/11] drm/mediatek: dp: Enable event interrupt only when bridge attached

[CK Hu (胡俊光)]

Hi, Angelo:

On Tue, 2023-07-25 at 09:32 +0200, AngeloGioacchino Del Regno wrote:
> It is useless and error-prone to enable the DisplayPort event
> interrupt
> before finishing to probe and install the driver, as the DP training
> cannot happen before the entire pipeline is correctly set up, as the
> interrupt handler also requires the full hardware to be initialized
> by
> mtk_dp_bridge_attach().
> 
> Anyway, depending in which state the controller is left from the
> bootloader, this may cause an interrupt storm and consequently hang
> the kernel during boot, so, avoid enabling the interrupt until we
> reach a clean state by adding the IRQ_NOAUTOEN flag before requesting
> it at probe time and manage the enablement of the ISR in the
> .attach()
> and .detach() handlers for the DP bridge.

Reviewed-by: CK Hu <ck.hu@xxxxxxxxxxxx>

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@xxxxxxxxxxxxx>
> Tested-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>
> Reviewed-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/mediatek/mtk_dp.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
> b/drivers/gpu/drm/mediatek/mtk_dp.c
> index e8d3bf310608..83e55f8dc84a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -100,6 +100,7 @@ struct mtk_dp_efuse_fmt {
>  struct mtk_dp {
>  	bool enabled;
>  	bool need_debounce;
> +	int irq;
>  	u8 max_lanes;
>  	u8 max_linkrate;
>  	u8 rx_cap[DP_RECEIVER_CAP_SIZE];
> @@ -2141,6 +2142,8 @@ 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);
>  
>  	return 0;
> @@ -2157,6 +2160,7 @@ 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);
>  	mtk_dp->drm_dev = NULL;
>  	mtk_dp_poweroff(mtk_dp);
>  	drm_dp_aux_unregister(&mtk_dp->aux);
> @@ -2475,7 +2479,7 @@ static int mtk_dp_probe(struct platform_device
> *pdev)
>  {
>  	struct mtk_dp *mtk_dp;
>  	struct device *dev = &pdev->dev;
> -	int ret, irq_num;
> +	int ret;
>  
>  	mtk_dp = devm_kzalloc(dev, sizeof(*mtk_dp), GFP_KERNEL);
>  	if (!mtk_dp)
> @@ -2484,9 +2488,9 @@ 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);
>  
> -	irq_num = platform_get_irq(pdev, 0);
> -	if (irq_num < 0)
> -		return dev_err_probe(dev, irq_num,
> +	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);
> @@ -2507,7 +2511,8 @@ static int mtk_dp_probe(struct platform_device
> *pdev)
>  
>  	spin_lock_init(&mtk_dp->irq_thread_lock);
>  
> -	ret = devm_request_threaded_irq(dev, irq_num, mtk_dp_hpd_event,
> +	irq_set_status_flags(mtk_dp->irq, IRQ_NOAUTOEN);
> +	ret = devm_request_threaded_irq(dev, mtk_dp->irq,
> mtk_dp_hpd_event,
>  					mtk_dp_hpd_event_thread,
>  					IRQ_TYPE_LEVEL_HIGH,
> dev_name(dev),
>  					mtk_dp);