Re: [PATCH v3 5/6] drm/mediatek: dsi: Add compatible for mt8167-dsi

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 03/03/26 08:56, CK Hu (胡俊光) ha scritto:
> On Wed, 2026-02-25 at 09:15 +0100, Luca Leonardo Scorcia wrote:
>> External email : Please do not click links or open attachments until you have verified the sender or the content.
>>
>>
>>>> The mt8167 DSI controller is fully compatible with the one found in
>>>> mt2701. Device tree documentation is already present upstream.
>>>
>>> If mt8167 DSI is fully compatible with mt2701 DSI, I think the binding document and device tree should be modified.
>>> In device tree,
>>>
>>>      compatible = "mediatek,mt8167-dsi", "mediatek,mt2701-dsi";
>>>
>>> And this patch is not necessary.
>>
>> Hi, if I understand your review correctly that's what v2 [1] of this
>> patch did, but the change was rejected during review.
>>
>> As far as I can see there is no win-win solution here. This tricky
>> situation derives from the fact that in last year's submissions the
>> change was only partially merged - the bindings went upstream while
>> the driver did not, and now we have to work around this. In v3 I tried
>> to address the issue by actually implementing what the binding
>> document says.
>> I'll be happy to resubmit v4 but I need to know what's the consensus here.
> 
> You mean Krzysztof's comment?
> He has two comment.
> The first is 'remove'. You actually not remove "mediatek,mt8167-dsi". You change it.
> The second is about fallback. I'm also not sure why he says it's NAK. Also a bad wording?
> After Angelo's comment, Krzysztof does not response.
> I also do not know what Krzysztof think.
> I agree with Angelo. Single "mediatek,mt8167-dsi" does not reflect the hardware compatible but fallback does.
> Maybe describe more information (hardware) to Krzysztof, and ask what's the better way to do.
> 

First of all: I would be happy with having

compatible = "mediatek,mt8167-dsi", "mediatek,mt2701-dsi";

in the devicetree, and to drop this patch - but that's not possible, check below.

The problem that Krzysztof was raising is that the bindings currently define
the "mediatek,mt8167-dsi" as single compatible with no fallback, and removing
it would break the ABI (as the bindings are not used only for Linux, but also
for bootloaders).

Even though I can try my best to convince him, this is a real problem: mistakes
were made in the past, and the only way to get the mt8167-dsi supported right
now is to, unfortunately, add the compatible in mtk_dsi.c.

I don't like that, and I see that you (CK) don't as well, but that's the result
of past mistakes... and apparently we can't do anything about this if not to
*uselessly clutter the driver* with that.

So, that said...

I'm sad to say that this patch is the only way to get this SoC probed by mtk_dsi
and that there's no other way around it, because the bindings maintainers are
forbidding us from removing the bad compatible (which was *never* used anyway)
from there and making it right with allowing it only under 2701 fallback.

I'm moving krzk to the To: field of this email in hope to get back a reply that
sounds like "you misunderstood me, you can do it", but I'm not positive about this.

Krzysztof, please confirm or deny what I said.

Regards,
Angelo


> Regards,
> CK
> 
>>
>> Thanks!
>>
>> [1] https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/ff920a7cc94f2b0c03d4bb55142030fded30d07c.1771258407.git.l.scorcia@gmail.com/__;!!CTRNKA9wMg0ARbw!lIBnCRkybM1oxoARb70JlLMPlXHKqW9dwSmVqXGsvj3LE9hsWuINkPgtrlr8Jl8-atOvzYoMjtavHOr3$
>>
>>>>
>>>> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
>>>> ---
>>>>   drivers/gpu/drm/mediatek/mtk_dsi.c | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
>>>> index af4871de9e4c..ad10e86b161d 100644
>>>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
>>>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
>>>> @@ -1301,6 +1301,7 @@ static const struct mtk_dsi_driver_data mt8188_dsi_driver_data = {
>>>>
>>>>   static const struct of_device_id mtk_dsi_of_match[] = {
>>>>          { .compatible = "mediatek,mt2701-dsi", .data = &mt2701_dsi_driver_data },
>>>> +       { .compatible = "mediatek,mt8167-dsi", .data = &mt2701_dsi_driver_data },
>>>>          { .compatible = "mediatek,mt8173-dsi", .data = &mt8173_dsi_driver_data },
>>>>          { .compatible = "mediatek,mt8183-dsi", .data = &mt8183_dsi_driver_data },
>>>>          { .compatible = "mediatek,mt8186-dsi", .data = &mt8186_dsi_driver_data },
>>>> --
>>>> 2.43.0
>>>>
>>>>
>> --
>> Luca Leonardo Scorcia
>> l.scorcia@gmail.com
>