Re: [PATCH v4 0/6] drm: lcdif: Add i.MX93 LCDIF support

From: Alexander Stein
Date: Fri Feb 17 2023 - 03:18:35 EST


Hi Liu,

Am Freitag, 17. Februar 2023, 07:54:01 CET schrieb Liu Ying:
> Hi,
>
> This patch set aims to add i.MX93 LCDIF display controller support
> in the existing LCDIF DRM driver. The LCDIF embedded in i.MX93 SoC
> is essentially the same to those embedded in i.MX8mp SoC. Through
> internal bridges, i.MX93 LCDIF may drive a MIPI DSI display or a LVDS
> display or a parallel display.
>
> Patch 1/6 adds device tree binding support for i.MX93 LCDIF in the
> existing fsl,lcdif.yaml.
>
> Patch 2/6 drops lcdif->bridge NULL pointer check as a cleanup patch.
>
> Patch 3/6~5/6 prepare for adding i.MX93 LCDIF support step by step.
>
> Patch 6/6 adds i.MX93 LCDIF compatible string as the last step of
> adding i.MX93 LCDIF support.

Thanks for the series. I could test this on my TQMa93xxLA/MBa93xxCA with a
single LVDS display attached, so no DSI or parallel display. Hence I could not
test the bus format and flags checks, but they look okay.
So you can add
Tested-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
to the whole series as well.

One thing I noticed is that, sometimes it seems that before probing lcdif my
system completely freezes. Adding some debug output it seems that's during
powering up the IMX93_MEDIABLK_PD_LCDIF power domain there is some race
condition. But adding more more detailed output made the problem go away.
Did you notice something similar? It might be a red hering though.

Best regards,
Alexander

>
> v3->v4:
> * Improve warning message when ignoring invalid LCDIF OF endpoint ids in
> patch 5/6. (Alexander)
> * Use 'new_{c,p}state' instead of 'new_{crtc,plane}_state' in patch 3/6.
> (Alexander)
> * Simplify lcdif_crtc_reset() by calling lcdif_crtc_atomic_destroy_state()
> in patch 3/6. (Alexander)
> * Add '!crtc->state' check in lcdif_crtc_atomic_duplicate_state() in patch
> 3/6. (Alexander)
> * Collect Alexander's R-b tags on patch 1/6, 2/6 and 6/6.
>
> v2->v3:
> * Fix a trivial typo in patch 6/6's commit message.
>
> v1->v2:
> * Add Krzysztof's A-b and Marek's R-b tags on patch 1/6.
> * Split patch 2/2 in v1 into patch 2/6~6/6 in v2. (Marek, Alexander)
> * Drop '!remote ||' from lcdif_attach_bridge(). (Lothar)
> * Add comment on the 'base' member of lcdif_crtc_state structure to
> note it should always be the first member. (Lothar)
> * Drop unneeded 'bridges' member from lcdif_drm_private structure.
> * Drop a comment about bridge input bus format from
> lcdif_crtc_atomic_check().
>
> Liu Ying (6):
> dt-bindings: lcdif: Add i.MX93 LCDIF support
> drm: lcdif: Drop unnecessary NULL pointer check on lcdif->bridge
> drm: lcdif: Determine bus format and flags in ->atomic_check()
> drm: lcdif: Check consistent bus format and flags across first bridges
> drm: lcdif: Add multiple encoders and first bridges support
> drm: lcdif: Add i.MX93 LCDIF compatible string
>
> .../bindings/display/fsl,lcdif.yaml | 7 +-
> drivers/gpu/drm/mxsfb/lcdif_drv.c | 71 ++++++-
> drivers/gpu/drm/mxsfb/lcdif_drv.h | 5 +-
> drivers/gpu/drm/mxsfb/lcdif_kms.c | 198 ++++++++++++------
> 4 files changed, 206 insertions(+), 75 deletions(-)


--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/