Re: [PATCH v4 3/3] drm/mediatek: Implement OF graphs support for display paths

Michael Walle
2024-06-03T07:42:57Z

[-- Attachment #1.1: Type: text/plain, Size: 1500 bytes --]

Hi Angelo,

> >> Implement OF graphs support to the mediatek-drm drivers, allowing to
> >> stop hardcoding the paths, and preventing this driver to get a huge
> >> amount of arrays for each board and SoC combination, also paving the
> >> way to share the same mtk_mmsys_driver_data between multiple SoCs,
> >> making it more straightforward to add support for new chips.
> >
> > paths might be optional, see comment in mtk_drm_kms_init(). But with
> > this patch, you'll get an -EINVAL with a disabled path. See my
> > proposals how to fix that below.
>
> I might not be understanding the reason behind allowing that but, per my logic, if
> a board does have a path, then it's written in devicetree and enabled - otherwise,
> it should not be there at all, in principle.
>
>
> Can you explain a bit more extensively the reason(s) why we need to account
> for disabled paths?

Paths should be (and this was already supported before this patch
with the hardcoded paths) disabled with the status property. This
way you can have a common board configuration where all the paths
are already described but are disabled. An overlay (or maybe another
dts variant) can then just enable the pipeline/output port by
overwriting the status property.

Also, this is the usual DT usage, as a node with status = "disabled"
should just be skipped. Without handling this, the current code will
return -EINVAL during probe (IIRC, my vacation might have reset my
memory :o).

-michael

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]