Re: [PATCH v1 17/17] drm/mediatek: Add mt8195-dpi support to drm_drv

On 05/10/2022 11:34, Guillaume Ranquet wrote:

On Tue, 04 Oct 2022 17:05, Krzysztof Kozlowski
[off-list ref] wrote:

quoted

On 04/10/2022 13:55, Guillaume Ranquet wrote:

quoted

quoted

No. You said what the code is doing. I think I understand this. You
still do not need more compatibles. Your sentence did not clarify it
because it did not answer at all to question "why". Why do you need it?

Sorry, the change looks not correct.

Best regards,
Krzysztof

I need a new compatible to adress the specifics of mt8195 in the mtk_dpi driver,
the change is in this series with:
[PATCH v1 16/17] drm/mediatek: dpi: Add mt8195 hdmi to DPI driver [1]

But you do not have specifics of mt8195. I wrote it in the beginning.

quoted

I then need to add that compatible to the "list" here in mtk_drm_drv.

No, you do not... I checked the driver and there is no single need... or
convince me you need.

quoted

I don't see a way around this unless I rewrite the way mtk_drm_drv works?

Why rewrite? You have all compatibles in place.

quoted

Maybe if I declare a new compatible that is generic to all mediatek
dpi variants?

You were asked to use fallback. Don't create some fake fallbacks. Use
existing ones.

quoted

and have all the dts specify the node with both the generic dpi and
the specific compatible?

dpi@xxx {
	compatible = "mediatek,dpi", "mediatek,mt8195-dpi";

I don't know what's this but certainly looks odd. Some wild-card
compatible in front (not fallback) and none are documented.

quoted

	...
}

Then I can "collapse" all the dpi related nodes in mtk_drm_drv under
"mediatek,dpi" ?

I guess would have to do the change for all other components that are needed in
mtk_drm_drv (mmsys, aal, ccor, color, dither, dsc, gamma, mutex...).

That's the only trivial way I can think of implementing this with the
current status
of the mtk_drm stack.

Do you have any other ideas in mind?

Use fallback of compatible device. That's the common pattern.
Everywhere, Mediatek as well.

I'm unsure about what a "fallback of compatible device" is.
But from what I can gather, you'd have me write the dts as:

dpi@xxx {
	compatible = "mediatek,mt8195-dpi", "mediatek,mt2701-dpi";
	...
}

Sounds reasonable. Just someone should check whether devices are
actually compatible, because driver is just a hint.

so that the mtk_dpi driver will use the specific mt8195 dpi config and
the mtk_drm driver will fallback to mt2701 to find the compatible it needs.

Would you like me to use this pattern for all the other compatibles declared
in the mtk_ddp_comp_ids array in separate patches?

Could be for consistency, but it is up to you.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel