linux-kernel - Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230825120109.3132209-1-mwalle@kernel.org>
Date:   Fri, 25 Aug 2023 14:01:09 +0200
From:   Michael Walle <mwalle@...nel.org>
To:     angelogioacchino.delregno@...labora.com
Cc:     airlied@...il.com, amergnat@...libre.com, chunkuang.hu@...nel.org,
        ck.hu@...iatek.com, daniel@...ll.ch,
        dri-devel@...ts.freedesktop.org, ehristev@...labora.com,
        kernel@...labora.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
        matthias.bgg@...il.com, nfraprado@...labora.com,
        p.zabel@...gutronix.de, wenst@...omium.org,
        Michael Walle <mwalle@...nel.org>
Subject: Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus

Hi AngeloGioacchino,

> For the eDP case we can support using aux-bus on MediaTek DP: this
> gives us the possibility to declare our panel as generic "panel-edp"
> which will automatically configure the timings and available modes
> via the EDID that we read from it.
> 
> To do this, move the panel parsing at the end of the probe function
> so that the hardware is initialized beforehand and also initialize
> the DPTX AUX block and power both on as, when we populate the
> aux-bus, the panel driver will trigger an EDID read to perform
> panel detection.
> 
> Last but not least, since now the AUX transfers can happen in the
> separated aux-bus, it was necessary to add an exclusion for the
> cable_plugged_in check in `mtk_dp_aux_transfer()` and the easiest
> way to do this is to simply ignore checking that when the bridge
> type is eDP.

This patch breaks my board based on the MT8195 which only has one
DisplayPort output port. I suspect it might also break the mt8195-cherry
board.

While the mediatek-dpi driver finds the DP port:
[    3.131645] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@...00000

The probing of the eDP is deferred:
[   13.289009] platform 1c015000.dp-intf: deferred probe pending

So I don't know why, but to make dp_intf1 work, it seems that dp_intf0
must be probed successfully. After this patch, the edp (which is
connected to the dp_intf1) probe will return with an -ENODEV and
the previous call to devm_drm_bridge_add() will be rolled back.

Before this patch, bridge_add() was called in any case (in the
error case with next_bridge = NULL) and the mediatek-dpi probed
like that:

[    3.121011] mediatek-dpi 1c015000.dp-intf: Found bridge node: /soc/edp-tx@...00000
[    3.122111] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@...00000

Eventually resulting in a framebuffer device:
[    4.451081] mediatek-drm mediatek-drm.8.auto: [drm] fb0: mediatekdrmfb frame buffer device


NB, somehow this series broke the initial display output. I always have
to replug the DisplayPort to get some output. I'll dig deeper into that
later.

..

> @@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev)
>  		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;

In my case, this branch was taken.

-michael

> -	else if (IS_ERR(mtk_dp->next_bridge))
> -		return dev_err_probe(dev, PTR_ERR(mtk_dp->next_bridge),
> -				     "Failed to get bridge\n");
> -
>  	ret = mtk_dp_dt_parse(mtk_dp, pdev);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to parse dt\n");
>  
> -	drm_dp_aux_init(&mtk_dp->aux);
>  	mtk_dp->aux.name = "aux_mtk_dp";
> +	mtk_dp->aux.dev = dev;
>  	mtk_dp->aux.transfer = mtk_dp_aux_transfer;
> +	drm_dp_aux_init(&mtk_dp->aux);
>  
>  	spin_lock_init(&mtk_dp->irq_thread_lock);
>