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

[Alexander Stein]

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/