Re: [PATCH v8 0/3] drm: Add support for Cadence MHDP DPI/DP bridge and J721E wrapper.

[Guido Günther]

Hi,
On Wed, Aug 12, 2020 at 01:47:42PM +0300, Tomi Valkeinen wrote:
> Hi Guido,
> 
> On 12/08/2020 11:39, Guido Günther wrote:
> > Hi,
> > On Thu, Aug 06, 2020 at 01:34:29PM +0200, Swapnil Jakhade wrote:
> >> This patch series adds new DRM bridge driver for Cadence MHDP DPI/DP
> >> bridge. The Cadence Display Port IP is also referred as MHDP (Mobile High
> >> Definition Link, High-Definition Multimedia Interface, Display Port).
> >> Cadence Display Port complies with VESA DisplayPort (DP) and embedded
> >> Display Port (eDP) standards.
> > 
> > Is there any relation to the cadence mhdp ip core used inthe imx8mq:
> > 
> >     https://lore.kernel.org/dri-devel/cover.1590982881.git.Sandor.yu@xxxxxxx/
> > 
> > It looks very similar in several places so should that use the same driver?
> > Cheers,
> >  -- Guido
> 
> Interesting.
> 
> So the original Cadence DP patches for TI SoCs did create a common driver with Rockchip's older mhdp
> driver. And looks like the IMX series points to an early version of that patch ("drm/rockchip:
> prepare common code for cdns and rk dpi/dp driver").
> 
> We gave up on that as the IPs did have differences and the firmwares used were apparently quite
> different. The end result was very difficult to maintain, especially as (afaik) none of the people
> involved had relevant Rockchip HW.

Is the `struct mhdp_platform_ops` a leftover from that? Can that be dropped?

> The idea was to get a stable DP driver for TI SoCs ready and upstream, and then carefully try to
> create common parts with Rockchip's driver in small pieces.

I wonder how imx8 would best blend into this? First thing will likely be
to upstream the phy code in driveres/phy/ so a modified version of this bridge
driver could call into that, then go and look for common patterns.

> If the Rockchip and IMX mhdp have the same IP and same firmware, then they obviously should share
> code as done in the series you point to.

I'm pretty sure they use different firmware though - the imx8mq
additionally supports HDMI with a different firmware on the same IP core
(13.4 and 13.5 in the imx8mq ref manual).

> Perhaps Cadence can clarify the differences between IMX, TI and
> Rockchip IPs and FWs?

That would be great!
 -- Guido


> I'm worried that if there are IP differences, even if not great ones, and if the FWs are different
> and developed separately, it'll be a constant "fix X for SoC A, and accidentally break Y for SoC B
> and C", especially if too much code is shared.
> 
> In the long run I'm all for a single driver (or large shared parts), but I'm not sure if we should
> start with that approach.




> 
>  Tomi
> 
> -- 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>