On Mon, 2023-02-20 at 11:16 +0100, Alexander Stein wrote: > Hi Liu, Hi Alexander, > > Am Montag, 20. Februar 2023, 09:55:19 CET schrieb Alexander Stein: > > Hi Liu, > > > > Am Freitag, 17. Februar 2023, 09:59:14 CET schrieb Liu Ying: > > > On Fri, 2023-02-17 at 09:18 +0100, Alexander Stein wrote: > > > > Hi Liu, > > > > > > Hi Alexander, > > > > > > > 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. > > > > > > Thanks for your test. > > > > > > > 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. > > > > > > I don't see system freezing with my i.MX93 11x11 EVK when probing > > > lcdif. I did try to boot the system several times. All look ok. > > > This is > > > a snippet of dmesg when lcdif probes: > > > > > > --------------------------8<------------------------------------- > > > ----- > > > [ 0.753083] Serial: 8250/16550 driver, 4 ports, IRQ sharing > > > enabled > > > [ 0.761669] SuperH (H)SCI(F) driver initialized > > > [ 0.766523] msm_serial: driver initialized > > > [ 0.780523] printk: console [ttyLP0] enabled0x44380010 (irq = > > > 16, > > > base_baud = 1500000) is a FSL_LPUART > > > [ 0.780523] printk: console [ttyLP0] enabled > > > [ 0.788928] printk: bootconsole [lpuart32] disabled > > > [ 0.788928] printk: bootconsole [lpuart32] disabled > > > [ 0.804632] panel-simple lvds_panel: supply power not found, > > > using > > > dummy regulator > > > [ 0.814741] [drm] Initialized imx-lcdif 1.0.0 20220417 for > > > 4ae30000.lcd-controller on minor 0 > > > [ 1.195930] Console: switching to colour frame buffer device > > > 160x50 > > > [ 1.218385] imx-lcdif 4ae30000.lcd-controller: [drm] fb0: imx- > > > lcdifdrmfb frame buffer device > > > [ 1.227099] cacheinfo: Unable to detect cache hierarchy for > > > CPU 0 > > > [ 1.236725] loop: module loaded > > > --------------------------8<------------------------------------- > > > ----- > > > > > > ~300 milliseconds are consumed by the enablement delay required > > > by the > > > "boe,ev121wxm-n10-1850" LVDS panel I use. > > > > It seems you have the drivers compiled in. I use modules in my case > > and > > simple-panel as well. But this is unrelated, because lcdif_probe() > > is yet to > > be called. Using the debug diff from below I get the following > > output: > > > > [ 16.111197] imx93-blk-ctrl 4ac10000.system-controller: > > imx93_blk_ctrl_power_on: 1 > > [ 16.122491] imx93-blk-ctrl 4ac10000.system-controller: > > imx93_blk_ctrl_power_on: 2 > > [ 16.137766] imx93-blk-ctrl 4ac10000.system-controller: > > imx93_blk_ctrl_power_on: 3 > > [ 16.154905] imx93-blk-ctrl 4ac10000.system-controller: > > imx93_blk_ctrl_power_on: 4 > > > > It seems setting BLK_CLK_EN blocks the whole system, even reading > > is not > > possible. I don't have any details on the hardware, but it seems > > that either > > some clock or power domain is not enabled. This can also happen if > > I'm > > loading the lcdif module manually after boot. But I can't detect > > any > > differences in / sys/kernel/debug/clk/clk_summary. > > I think I found the cause. It's the maximum clock frequency for > media_axi and > media_apb. These clocks were not explicitly configured, most > probably > exceeding the maximum frequency allowed. Thanks for sharing the cause. I use 400MHz media_axi and 133MHz media_apb. Regards, Liu Ying > > Best regards, > Alexander > > > ---8<--- > > diff --git a/drivers/soc/imx/imx93-blk-ctrl.c > > b/drivers/soc/imx/imx93-blk- > > ctrl.c > > index 2c600329436cf..50aeb20ce90dc 100644 > > --- a/drivers/soc/imx/imx93-blk-ctrl.c > > +++ b/drivers/soc/imx/imx93-blk-ctrl.c > > @@ -129,12 +129,14 @@ static int imx93_blk_ctrl_power_on(struct > > generic_pm_domain *genpd) > > struct imx93_blk_ctrl *bc = domain->bc; > > int ret; > > > > + dev_info(bc->dev, "%s: 1\n", __func__); > > ret = clk_bulk_prepare_enable(bc->num_clks, bc->clks); > > if (ret) { > > dev_err(bc->dev, "failed to enable bus clocks\n"); > > return ret; > > } > > > > + dev_info(bc->dev, "%s: 2\n", __func__); > > ret = clk_bulk_prepare_enable(data->num_clks, domain->clks); > > if (ret) { > > clk_bulk_disable_unprepare(bc->num_clks, bc->clks); > > @@ -142,6 +144,7 @@ static int imx93_blk_ctrl_power_on(struct > > generic_pm_domain *genpd) > > return ret; > > } > > > > + dev_info(bc->dev, "%s: 3\n", __func__); > > ret = pm_runtime_get_sync(bc->dev); > > if (ret < 0) { > > pm_runtime_put_noidle(bc->dev); > > @@ -149,11 +152,15 @@ static int imx93_blk_ctrl_power_on(struct > > generic_pm_domain *genpd) > > goto disable_clk; > > } > > > > + dev_info(bc->dev, "%s: 4\n", __func__); > > + > > /* ungate clk */ > > regmap_clear_bits(bc->regmap, BLK_CLK_EN, data->clk_mask); > > + dev_info(bc->dev, "%s: 5\n", __func__); > > > > /* release reset */ > > regmap_set_bits(bc->regmap, BLK_SFT_RSTN, data->rst_mask); > > + dev_info(bc->dev, "%s: 6\n", __func__); > > > > dev_dbg(bc->dev, "pd_on: name: %s\n", genpd->name); > > > > > > ---8<--- > > > > Best regards, > > Alexander > > > > > Regards, > > > Liu Ying > > > > > > > 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(-) > >