Re: [PATCH v4 2/2] drm: lcdif: Add support for i.MX8MP LCDIF variant

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi Marek,

Am Dienstag, 24. Mai 2022, 09:29:43 CEST schrieb Marek Vasut:
> On 5/24/22 09:09, Alexander Stein wrote:
> > Hi Marek,
> 
> Hi,
> 
> > Am Donnerstag, 19. Mai 2022, 13:48:49 CEST schrieb Marek Vasut:
> >> Add support for i.MX8MP LCDIF variant. This is called LCDIFv3 and is
> >> completely different from the LCDIFv3 found in i.MX23 in that it has
> >> a completely scrambled register layout compared to all previous LCDIF
> >> variants. The new LCDIFv3 also supports 36bit address space.
> >> 
> >> Add a separate driver which is really a fork of MXSFB driver with the
> >> i.MX8MP LCDIF variant handling filled in.
> >> 
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Cc: Lucas Stach <l.stach@pengutronix.de>
> >> Cc: Peng Fan <peng.fan@nxp.com>
> >> Cc: Robby Cai <robby.cai@nxp.com>
> >> Cc: Sam Ravnborg <sam@ravnborg.org>
> >> Cc: Stefan Agner <stefan@agner.ch>
> >> ---
> >> V2: - Drop the pitch check from lcdif_fb_create()
> >> 
> >>      - Drop connector caching
> >>      - Wait for shadow load bit to be cleared in IRQ handler
> >>      - Make all clock mandatory and grab them all by name
> >>      - Wait for EN to be cleared in lcdif_disable_controller
> >>      - Rename to imx-lcdif
> >>      - Move shadow load to atomic_flush
> >> 
> >> V3: - Invert DE polarity to match MX8MPRM datasheet
> >> 
> >>      - Enable CSC in RGB to YUV mode for MEDIA_BUS_FMT_UYVY8_1X16
> >> 
> >> V4: - Drop lcdif_overlay_plane_formats, it is unused
> > 
> > Thanks for the update. With your change in V3 my HDMI output works now
> > without that hack mentioned. weston screen as well as 'fb-test -p 5'
> > output seems sensible.
> > Unfortunately this isn't the case for LVDS output on LCDIF2. I somehow
> > managed to get the DT nodes for LCDIF and LDB done. Also the necessary
> > addition to imx8m-blk-ctl. So eventually I can see some output. But the
> > screen is cutoff on the right side of about 15-20% and the screen is
> > flickering slighty. This is especially visible in 'fb-test -p 5'. The red
> > bars are only visible to less than 1/3 and the text as well as the
> > diagonal lines are flickering. Colors are correct though.
> > For the record: I am using a 'tianma,tm070jvhg33' panel.
> 
> Does LDB start working if you apply:
> 
>   static const struct drm_bridge_funcs funcs = {
>          .attach = fsl_ldb_attach,
> -       .atomic_check = fsl_ldb_atomic_check,
>          .atomic_enable = fsl_ldb_atomic_enable,
>          .atomic_disable = fsl_ldb_atomic_disable,
>          .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> 
> to
> 
> drivers/gpu/drm/bridge/fsl-ldb.c

I got this working, somehow. The root cause was that the LDB clock was not the 
media_disp2_pix_root_clk clock * 7, which is mandatory for LVDS (single link).

excerpt from clk_summary:
video_pll1_out     2 2 0   594000000          0     0  50000         Y
   media_ldb       1 1 0   594000000          0     0  50000         Y
   media_disp2_pix 1 1 0    66000000          0     0  50000         Y

media_ldb is too high (should be 462000000). I wonder why media_ldb is not a 
child from media_disp2_pix (or vice versa) when there is a hard dependency.
There are several solutions:
1.
Set video_pll1 to 1039500000 and adjust requested pixel clock of the panel 
(74250000 in this case). Now the dividers match hit the clock rates exactly.
But this renders the display list in panel-simple a bit useless.

2.
Adjust video_pll1_out only (e.g. 478757145). Now the calculated clocks comply 
to their mandated ratio. But this might affect other users, e.g. DSI displays

3.
Improve fsl_ldb_atomic_check to set adjusted_mode.clock to an achievable 
clock. This way lcdif will pick the new pixelclock to match their ratio.
But there is more work necessary, e.g. ensure the new pixelclock is in the
valid range of the display.

To summarize:
For both HDMI and LVDS using changes unrelated to this lcdif driver:
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>