Re: [EXT] Re: [PATCH 0/2] drm: imx: Add NWL MIPI DSI host controller support

[Laurentiu Palcu <laurentiu.palcu@nxp.com>]

Lucas and Daniel,

On Tue, May 28, 2019 at 10:36:43AM +0200, Daniel Vetter wrote:
> Caution: EXT Email
> 
> On Tue, May 28, 2019 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> >
> > Hi Shawn,
> >
> > Am Dienstag, den 28.05.2019, 09:38 +0800 schrieb Shawn Guo:
> > > Hi Lucas,
> > >
> > > On Mon, May 27, 2019 at 03:36:53PM +0200, Lucas Stach wrote:
> > > > We have been looking at how to support DCSS in mainline for a while,
> > > > but most of the actual work got pushed behind in schedule due to other
> > > > priorities.
> > >
> > > I have some time to contribute.  Would you suggest how I should help
> > > here?
> > >
> > > 1. You guys already have something close to completion and do not need
> > >    more hands.
> > > 2. You guys already have some preliminary code and can use help from
> > >    others.
> > > 3. You guys haven't got anything to start with, but just some design
> > >    principles that anyone who wants to work on it should consider.
> > >
> > > Which is the one that you want me to read?
> >
> > Mostly the 3rd. We spent some time on getting the DCSS driver to work
> > on upstream kernel and familiarize with the hardware, but we don't have
> > any close to mainline quality code.
> >
> > > > One thing I can can say for certain is that DCSS should not be
> > > > integrated into imx-drm. It's a totally different hardware and
> > > > downstream clearly shows that it's not a good idea to cram it into imx-
> > > > drm.
> > >
> > > I haven't gone deeper into the vendor code, but from a brief looking I
> > > didn't see so many problems with integrating DCSS into imx-drm.  It's
> > > not so unreasonable to take imx-drm as an imx-display-subsystem which
> > > can have multiple CRTCs.  So can you please elaborate a bit on why it's
> > > really a bad idea?
> >
> > It's a totally different hardware, with very different behavior, so
> > there is basically no potential for any code sharing. The downstream
> > driver is a hell of "oh, things are different here, let's introduce yet
> > another function pointer to make the distinction between the HW". It
> > complicates the imx-drm for no good reason. Our experience with imx-drm
> > is that it is already at a complexity level that makes it hard to
> > reason about things when hunting bugs.
> >
> > The boiler plate required to write a atomic KMS driver is not that
> > much, so I would rather have a clean split between the two hardware
> > drivers. Frankly they don't share anything except both being a atomic
> > KMS driver and running on a SoC called i.MX.
> 
> We've also made lots of improvements in the helpers, I think a clean
> driver will be quiet a bit smaller than an imx based one. ARM is doing
> the same with komeda and the malidp driver btw. Another option would
> be 2 kms drivers in one .ko, which is what nouveau does with pre-nv50
> and post-nv50. But that only makes sense if you have also the render
> side in the same driver because it's all from the same vendor. msm is
> similar, with msm4 vs msm5.
> 
> But for soc display-only I think two separate drivers, if the hw
> really changed enough, is the best option. You can still stuff them
> into the same subdir ofc.

Sounds good to me. I'll rewrite the DCSS driver and make it separate
from imx-drm. Though, to be honest, this defeats the whole purpose of
imx-drm in the first place... Wasn't this supposed to act like a glue
and gather all i.MX related display controllers under the imx-drm
umbrella?

But, on the other hand, I agree it would be best, going forward, to have
it separate. Easier to maintain and, most likely, simpler.

thanks,
laurentiu

> 
> Cheers, Daniel
> 
> > > > Also the artificial split between hardware control in
> > > > drivers/gpu/imx/dcss and the DRM driver is just cargo-cult from the
> > > > IPU/imx-drm split. For the IPU we did it as the IPU has legs in both
> > > > DRM for the output parts and V4L2 for the input parts. As the DCSS has
> > > > no video input capabilities the driver could be simplified a lot by
> > > > moving it all into a single DRM driver.
> > >
> > > Agreed on this.
> >
> > Regards,
> > Lucas
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Claurentiu.palcu%40nxp.com%7C8dca19419c7e49bd10f608d6e347a6e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946294240955699&amp;sdata=jaSgJsk2LinqaCbUxe6aIvkM6oasWFgezfTZMEMo5Uo%3D&amp;reserved=0
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch&amp;data=02%7C01%7Claurentiu.palcu%40nxp.com%7C8dca19419c7e49bd10f608d6e347a6e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946294240965691&amp;sdata=ObCThdTNsTYvJ75nnLQe4G7HToiIFseQgJoaeljZn6M%3D&amp;reserved=0
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Claurentiu.palcu%40nxp.com%7C8dca19419c7e49bd10f608d6e347a6e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946294240965691&amp;sdata=n9a86mh1apiyDgv3rSJ7G3fz1k22x%2B%2Bu31uxZAI9N0Q%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel