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

[Swapnil Kashinath Jakhade]

Hi Guido,

> -----Original Message-----
> From: Guido Günther <agx@xxxxxxxxxxx>
> Sent: Wednesday, August 12, 2020 7:27 PM
> To: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> Cc: Swapnil Kashinath Jakhade <sjakhade@xxxxxxxxxxx>; airlied@xxxxxxxx;
> daniel@xxxxxxxx; Laurent.pinchart@xxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx;
> a.hajda@xxxxxxxxxxx; narmstrong@xxxxxxxxxxxx; jonas@xxxxxxxxx;
> jernej.skrabec@xxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Milind Parab
> <mparab@xxxxxxxxxxx>; Yuti Suresh Amonkar <yamonkar@xxxxxxxxxxx>;
> praneeth@xxxxxx; nsekhar@xxxxxx; jsarha@xxxxxx; sandor.yu@xxxxxxx
> Subject: Re: [PATCH v8 0/3] drm: Add support for Cadence MHDP DPI/DP
> bridge and J721E wrapper.
> 
> EXTERNAL MAIL
> 
> 
> 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://urldefense.com/v3/__https://lore.kernel.org/dri-devel/cover.
> > >
> 1590982881.git.Sandor.yu@xxxxxxx/__;!!EHscmS1ygiU1lA!QIVUQ0JEY1Wz4
> gM
> > > qV3HYGyyp5m4r_Fje6dL5ptUdhSzeqzzqBBR0Jo-BC9arK-g$
> > >
> > > 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
> 

Following are the differences between MHDP IPs from Cadence for Rockchip, TI and NxP:

The Rockchip and NXP MHDP Core shares the same part (IP8501) which is DP v1.3 SST
Controller with HDCP 2.2/1.x. NXP's version additionally supports HDMI.
TI uses a different part (IP8546A), which is DP v1.4 with HDCP 2.2/1.x.
TI DP Controller adds support for additional features such as Multi Stream Support (MST),
Forward Error Correction (FEC) and Compression (DSC).

Also, FW used for TI has significant differences than FW used for Rockchip or NXP.
NxP and TI firmware are developed and maintained separately by Cadence and are in
active support.

>From the Linux driver perspective, given the differences, it would make sense to have
TI driver maintained separately.

Thanks,
Swapnil

> 
> > 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
> >