Re: [PATCH 0/8] Add imx8mp video support

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Michael,

michael@amarulasolutions.com wrote on Tue, 10 Sep 2024 12:30:42 +0200:

> Hi Miquel
> 
> On Tue, Sep 10, 2024 at 12:13 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > In order to display a boot picture or an error message, the i.MX8MP
> > display pipeline must be enabled. The SoC has support for various
> > interfaces (LVDS, HDMI, DSI). The one supported in this series is the
> > standard 4-lane LVDS output. The minimal setup is thus composed of:
> > * An LCD InterFace (LCDIF) with an AXI/APB interface, generating a pixel
> >   stream
> > * One LVDS Display Bridge (LDB), also named pixel mapper, which receives
> >   the pixel stream and route it to one or two (possibly combined) LVDS
> >   displays.
> > * All necessary clocks and power controls coming from the MEDIAMIX
> >   control block.
> >
> > Patch 1 adds a very useful helper to the core in order to grab devices
> > through endpoints instead of being limited to phandles. Video pipelines
> > being often described using graphs endpoints, the introduced helper is
> > used several times in the serires (there are 3 LCDIF, one of them being
> > connected to the LDB, itself having 2 ports).
> >
> > Patch 2 is a fix which is necessary for this series to work properly,
> > but is way broader than just this use case. In practice, when assigned
> > clocks are defined in the device tree, the clock uclass tries to assign
> > the parents first and then sets them to the correct frequency. This only
> > works if the parents have been enabled themselves. Otherwise we end-up
> > with a non-clocked parent. I believe this is not the intended behavior
> > in general, but more importantly on the i.MX8MP, there are "clock
> > slices" which have pre-requisites in order to be modified and selecting
> > an ungated parent is one of them.
> >
> > All the other patches progressively build support for the whole video
> > pipeline. Regarding the LCDIF driver, there is already a similar driver
> > for older i.MX SoCs but I didn't manage to get it to work. It was
> > written more than a decade ago while device-model, clocks and others
> > were not yet generically supported. Thus, numerous ad-hoc solutions
> > were implemented, which no longer fit today's requirements. I preferred
> > to add a new "clean" driver instead of modifying the existing one
> > because of the too high risk of breaking these platforms. Once proper
> > clocks/power-domain descriptions will be added to them they might be
> > converted (and tested) to work with the "new" implementation, but going
> > the opposite way felt drawback.
> >  
> 
> Thank you for adding those patches. We are working on mipi support
> here and some of the clock patches
> are there too. I will try to look and rebase our patchset
> 
> https://patchwork.amarulasolutions.com/patch/3401/

Thanks for letting me know. Indeed there are a couple of conflicts.
- My patch 2 can easily be replaced by your approach. 
- Regarding the power domain we had two different approaches, I
  didn't look in details, but again one or the other seems fine, I
  guess.
- The LDB driver in my series is new.
- Finally regarding the LCDIF changes, the approach taken on your
  side follows the somewhat too ad-hoc logic and would probably benefit
  from being migrated to the driver I propose, which does make use of
  the (DM) clock, power domain and DT API. I am really happy with the
  core helper I've added retrieving a device in front of a graph
  endpoint, which makes the whole "get panel/bridge" approach much more
  flexible and adapted to today's needs.

What is the status of your patchset? How shall we handle the conflicts?

Thanks,
Miquèl