| Message ID | cover.1551954425.git.agx@sigxcpu.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: imx: Add NWL MIPI DSI host controller support | expand |
Hi, On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote: > This adds initial support for the NWL MIPI DSI Host controller found on i.MX8 > SoCs. > > It adds support for the i.MX8MQ but the same IP core can also be found on e.g. > i.MX8QXP. I added the necessary hooks to support other imx8 variants but since > I only have imx8mq boards to test I omitted the platform data for other SoCs. > > The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by but > I'm happy to swap Author: and Co-authored-by: if that looks more appropriate. > The most notable changes over the BSP driver are > - Calculate HS mode timing from phy_configure_opts_mipi_dphy > - Perform all clock setup via DT > - Merge nwl-imx and nwl drivers > - Add B0 silion revision quirk > > Posting this is likely a bit premature (hence v0) but I wanted for one show how > this hooks into the mixel dphy posted earlier [1] and avoid duplicating work. > So if there's other code out there doing the same I'm be happy to merge > efforts. Since this is likely not going anywhere until we have a dcss driver aimed for mainline I'm not going spam the list with further revisions. However the 5.x version is maintained here: https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip Feedback is still welcome. It'll eventually be forwarded to newer linux-next versions. Changes over the posted version are: - Add quirk for IMQ8MQ silicon B0 revision to not mess with the system reset controller on power down since enable won't work afterwards otherwise. - Disable tx esc clock *after* the phy power down to unbreak disable/enable (unblank/blank) - Drop devm_free_irq() handled by the device driver core - Add ports to dt binding docs - Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for phy_mipi_dphy_get_default_config - Include drm_print.h to fix build on next-20190408 - Drop some debugging messages - Newline terminate all DRM_ printouts If somebody is working on DCSS support it'd be cool to know since this driver is currently a component of imx-display-subsystem which will only work out if dcss is handled like this as well. Cheers, -- Guido > > It has been tested quite bit (in a version backported to 4.18) on Librem 5 > devkit using DCSS (which is not mainlined yet) and a MIPI DSI panel[2]. In > principle LCDIF can also act as input source. I intend look into next so this > can actually be tested without further patches on mainline kernels. > > [1]: https://lists.freedesktop.org/archives/dri-devel/2019-March/209680.html > [2]: https://source.puri.sm/guido.gunther/linux-imx8/tree/imx8-4.18-wip-nwl-dsi-rework > > Guido Günther (2): > dt-bindings: imx: Add binding for IMX NWL mipi dsi host controller > drm/imx: Add NWL MIPI DSI host controller support > > .../bindings/display/imx/imx-nwl-dsi.txt | 72 ++ > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/nwl/Kconfig | 12 + > drivers/gpu/drm/nwl/Makefile | 2 + > drivers/gpu/drm/nwl/nwl-drv.c | 594 ++++++++++++++ > drivers/gpu/drm/nwl/nwl-drv.h | 68 ++ > drivers/gpu/drm/nwl/nwl-dsi.c | 752 ++++++++++++++++++ > drivers/gpu/drm/nwl/nwl-dsi.h | 105 +++ > 9 files changed, 1608 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/imx/imx-nwl-dsi.txt > create mode 100644 drivers/gpu/drm/nwl/Kconfig > create mode 100644 drivers/gpu/drm/nwl/Makefile > create mode 100644 drivers/gpu/drm/nwl/nwl-drv.c > create mode 100644 drivers/gpu/drm/nwl/nwl-drv.h > create mode 100644 drivers/gpu/drm/nwl/nwl-dsi.c > create mode 100644 drivers/gpu/drm/nwl/nwl-dsi.h > > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, May 08, 2019 at 07:18:27PM +0200, Guido Günther wrote: > If somebody is working on DCSS support it'd be cool to know since this I have some time slots here and will start looking at it, if no one else is already working on it. Shawn > driver is currently a component of imx-display-subsystem which will only > work out if dcss is handled like this as well.
Hi, On Mon, May 27, 2019 at 10:24:02AM +0800, Shawn Guo wrote: > On Wed, May 08, 2019 at 07:18:27PM +0200, Guido Günther wrote: > > If somebody is working on DCSS support it'd be cool to know since this > > I have some time slots here and will start looking at it, if no one else > is already working on it. Nice. My current forward port of a minimal DCSS needed for DSI is here: https://source.puri.sm/guido.gunther/linux-imx8/tree/imx8-5.x-drm Cheers, -- Guido
Am Mittwoch, den 08.05.2019, 19:18 +0200 schrieb Guido Günther: > Hi, > On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote: > > This adds initial support for the NWL MIPI DSI Host controller found on i.MX8 > > SoCs. > > > > It adds support for the i.MX8MQ but the same IP core can also be found on e.g. > > i.MX8QXP. I added the necessary hooks to support other imx8 variants but since > > I only have imx8mq boards to test I omitted the platform data for other SoCs. > > > > The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by but > > I'm happy to swap Author: and Co-authored-by: if that looks more appropriate. > > The most notable changes over the BSP driver are > > - Calculate HS mode timing from phy_configure_opts_mipi_dphy > > - Perform all clock setup via DT > > - Merge nwl-imx and nwl drivers > > - Add B0 silion revision quirk > > > > Posting this is likely a bit premature (hence v0) but I wanted for one show how > > this hooks into the mixel dphy posted earlier [1] and avoid duplicating work. > > So if there's other code out there doing the same I'm be happy to merge > > efforts. > > Since this is likely not going anywhere until we have a dcss driver aimed > for mainline I'm not going spam the list with further revisions. However > the 5.x version is maintained here: > > https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip > > Feedback is still welcome. It'll eventually be forwarded to newer > linux-next versions. > > Changes over the posted version are: > > - Add quirk for IMQ8MQ silicon B0 revision to not mess with the > system reset controller on power down since enable won't work > afterwards otherwise. > - Disable tx esc clock *after* the phy power down to unbreak > disable/enable (unblank/blank) > - Drop devm_free_irq() handled by the device driver core > - Add ports to dt binding docs > - Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for > phy_mipi_dphy_get_default_config > - Include drm_print.h to fix build on next-20190408 > - Drop some debugging messages > - Newline terminate all DRM_ printouts > > If somebody is working on DCSS support it'd be cool to know since this > driver is currently a component of imx-display-subsystem which will only > work out if dcss is handled like this as well. 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. 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. 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. Regards, Lucas
Hi Lucas, On Mon, May 27, 2019 at 03:36:53PM +0200, Lucas Stach wrote: > Am Mittwoch, den 08.05.2019, 19:18 +0200 schrieb Guido Günther: > > Hi, > > On Thu, Mar 07, 2019 at 11:30:51AM +0100, Guido Günther wrote: > > > This adds initial support for the NWL MIPI DSI Host controller found on i.MX8 > > > SoCs. > > > > > > It adds support for the i.MX8MQ but the same IP core can also be found on e.g. > > > i.MX8QXP. I added the necessary hooks to support other imx8 variants but since > > > I only have imx8mq boards to test I omitted the platform data for other SoCs. > > > > > > The code is based on NXPs BSP so I added Robert Chiras as Co-authored-by but > > > I'm happy to swap Author: and Co-authored-by: if that looks more appropriate. > > > The most notable changes over the BSP driver are > > > - Calculate HS mode timing from phy_configure_opts_mipi_dphy > > > - Perform all clock setup via DT > > > - Merge nwl-imx and nwl drivers > > > - Add B0 silion revision quirk > > > > > > Posting this is likely a bit premature (hence v0) but I wanted for one show how > > > this hooks into the mixel dphy posted earlier [1] and avoid duplicating work. > > > So if there's other code out there doing the same I'm be happy to merge > > > efforts. > > > > Since this is likely not going anywhere until we have a dcss driver aimed > > for mainline I'm not going spam the list with further revisions. However > > the 5.x version is maintained here: > > > > https://source.puri.sm/guido.gunther/linux-imx8/tree/forward-upstream/next-20190506/imx-nwl/v1-wip > > > > Feedback is still welcome. It'll eventually be forwarded to newer > > linux-next versions. > > > > Changes over the posted version are: > > > > - Add quirk for IMQ8MQ silicon B0 revision to not mess with the > > system reset controller on power down since enable won't work > > afterwards otherwise. > > - Disable tx esc clock *after* the phy power down to unbreak > > disable/enable (unblank/blank) > > - Drop devm_free_irq() handled by the device driver core > > - Add ports to dt binding docs > > - Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for > > phy_mipi_dphy_get_default_config > > - Include drm_print.h to fix build on next-20190408 > > - Drop some debugging messages > > - Newline terminate all DRM_ printouts > > > > If somebody is working on DCSS support it'd be cool to know since this > > driver is currently a component of imx-display-subsystem which will only > > work out if dcss is handled like this as well. > > 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. > > 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. > > 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. I agree. While moving if forward from NXPs tree this caused more trouble than good so let's keep it separate form imx-drm. Cheers, -- Guido
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? > 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? > 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. Shawn
Hi Shawn, Lucas, On Tue, May 28, 2019 at 09:38:02AM +0800, Shawn Guo wrote: > Caution: EXT Email > > 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? We're already working on clearing up the DCSS code and preparing it for upstreaming. It should be done in the following weeks. The reason we've been delaying this is because neither HDMI nor MIPI support was present and, until these are upstream, testing DCSS would be quite impossible. > > > 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? I'd be interested to hear about this as well. > > > 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. I also agree on this. DCSS core code will probably be moved inside the same directory: drivers/gpu/drm/imx/dcss. Thanks, laurentiu > > Shawn > _______________________________________________ > 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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7Cda7e62c6b69f4e0c800408d6e30d4dfc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946043619737103&sdata=bnr9EJG5y4Hqr%2FUT5T3EfvWIQKAvkVCZGhdPwEPJQOw%3D&reserved=0
Hi Laurentiu, On Tue, May 28, 2019 at 07:03:54AM +0000, Laurentiu Palcu wrote: > Hi Shawn, Lucas, > > On Tue, May 28, 2019 at 09:38:02AM +0800, Shawn Guo wrote: > > Caution: EXT Email > > > > 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? > > We're already working on clearing up the DCSS code and preparing it for > upstreaming. It should be done in the following weeks. Great! I will stay away from this then :) > The reason we've > been delaying this is because neither HDMI nor MIPI support was present > and, until these are upstream, testing DCSS would be quite impossible. Well, they have to be done one by one, and I guess DCSS should be the first one. I think it's fine to test DCSS with out-of-tree HDMI or MIPI driver which is not ready for submission yet. Shawn
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. > > 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
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. 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://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Laurentiu, On Tue, May 28, 2019 at 07:03:54AM +0000, Laurentiu Palcu wrote: > Hi Shawn, Lucas, > > On Tue, May 28, 2019 at 09:38:02AM +0800, Shawn Guo wrote: > > Caution: EXT Email > > > > 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? > > We're already working on clearing up the DCSS code and preparing it for > upstreaming. It should be done in the following weeks. The reason we've > been delaying this is because neither HDMI nor MIPI support was present > and, until these are upstream, testing DCSS would be quite impossible. MIPI support is here: mixel: https://patchwork.freedesktop.org/series/58817/ nwl: https://patchwork.freedesktop.org/series/57686/ The NWL driver needs to be adjusted depending on whether we hook into imx-display-subsystem or not (and then likely moved to the right subdir). Can we somehow get this moving in sync (even in a non public tree if necessary). Cheers, -- Guido > > > 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? > > I'd be interested to hear about this as well. > > > > > > 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. > > I also agree on this. DCSS core code will probably be moved inside the > same directory: drivers/gpu/drm/imx/dcss. > > Thanks, > laurentiu > > > > > Shawn > > _______________________________________________ > > 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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7Cda7e62c6b69f4e0c800408d6e30d4dfc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946043619737103&sdata=bnr9EJG5y4Hqr%2FUT5T3EfvWIQKAvkVCZGhdPwEPJQOw%3D&reserved=0 > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C8dca19419c7e49bd10f608d6e347a6e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946294240955699&sdata=jaSgJsk2LinqaCbUxe6aIvkM6oasWFgezfTZMEMo5Uo%3D&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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C8dca19419c7e49bd10f608d6e347a6e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946294240965691&sdata=ObCThdTNsTYvJ75nnLQe4G7HToiIFseQgJoaeljZn6M%3D&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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C8dca19419c7e49bd10f608d6e347a6e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946294240965691&sdata=n9a86mh1apiyDgv3rSJ7G3fz1k22x%2B%2Bu31uxZAI9N0Q%3D&reserved=0
Hi Guido, On Tue, May 28, 2019 at 11:33:00AM +0200, Guido Günther wrote: > Caution: EXT Email > > Hi Laurentiu, > On Tue, May 28, 2019 at 07:03:54AM +0000, Laurentiu Palcu wrote: > > Hi Shawn, Lucas, > > > > On Tue, May 28, 2019 at 09:38:02AM +0800, Shawn Guo wrote: > > > Caution: EXT Email > > > > > > 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? > > > > We're already working on clearing up the DCSS code and preparing it for > > upstreaming. It should be done in the following weeks. The reason we've > > been delaying this is because neither HDMI nor MIPI support was present > > and, until these are upstream, testing DCSS would be quite impossible. > > MIPI support is here: > > mixel: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F58817%2F&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839828366&sdata=VZvvhe2WkMVCSOEw5oZDJfy7rsqF7YaEirrkLFC8Icw%3D&reserved=0 > nwl: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F57686%2F&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=Q4s6OZq1KElktjSXRd2lKBMdg1yPJsWGm8UrSPqqTiE%3D&reserved=0 > > The NWL driver needs to be adjusted depending on whether we hook into > imx-display-subsystem or not (and then likely moved to the right > subdir). Can we somehow get this moving in sync (even in a non public > tree if necessary). I guess we could do that as well. I'll start adjusting the driver and take it out of imx-drm, as suggested by Lucas and Daniel. I'll use your MIPI patches to test with. thanks, laurentiu > Cheers, > -- Guido > > > > > > 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? > > > > I'd be interested to hear about this as well. > > > > > > > > > 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. > > > > I also agree on this. DCSS core code will probably be moved inside the > > same directory: drivers/gpu/drm/imx/dcss. > > > > Thanks, > > laurentiu > > > > > > > > Shawn > > > _______________________________________________ > > > 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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=6I8MCXrt3y4nX20SnpfoSwEZkg%2B1zP3AFLGHUNaI%2Fls%3D&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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=6I8MCXrt3y4nX20SnpfoSwEZkg%2B1zP3AFLGHUNaI%2Fls%3D&reserved=0
Hi Laurentiu, Am Dienstag, den 28.05.2019, 10:04 +0000 schrieb Laurentiu Palcu: > Lucas and Daniel, [...] > > > 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? Not really. We never had a plan to support all kinds of i.MX display hardware in a single driver. At some point imx-drm acted as the glue to bring all the components of the i.MX display pipeline together, but much of that functionality has since been split out and is now available as infrastructure in the kernel, like the DRM bridges, devicetree graph bindings and device links. All those infrastructure parts make it easy to write a separate DRM driver, without duplicating lots of code from imx-drm. It made sense to extend imx-drm as long as the hardware was close and/or an extension to already existing hardware. But DCSS is totally different from the currently supported IPU design. > But, on the other hand, I agree it would be best, going forward, to have > it separate. Easier to maintain and, most likely, simpler. Seems we agree on the direction this should take. :) Regards, Lucas
Hi, On Tue, May 28, 2019 at 10:10:11AM +0000, Laurentiu Palcu wrote: > Hi Guido, > > On Tue, May 28, 2019 at 11:33:00AM +0200, Guido Günther wrote: > > Caution: EXT Email > > > > Hi Laurentiu, > > On Tue, May 28, 2019 at 07:03:54AM +0000, Laurentiu Palcu wrote: > > > Hi Shawn, Lucas, > > > > > > On Tue, May 28, 2019 at 09:38:02AM +0800, Shawn Guo wrote: > > > > Caution: EXT Email > > > > > > > > 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? > > > > > > We're already working on clearing up the DCSS code and preparing it for > > > upstreaming. It should be done in the following weeks. The reason we've > > > been delaying this is because neither HDMI nor MIPI support was present > > > and, until these are upstream, testing DCSS would be quite impossible. > > > > MIPI support is here: > > > > mixel: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F58817%2F&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839828366&sdata=VZvvhe2WkMVCSOEw5oZDJfy7rsqF7YaEirrkLFC8Icw%3D&reserved=0 > > nwl: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F57686%2F&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=Q4s6OZq1KElktjSXRd2lKBMdg1yPJsWGm8UrSPqqTiE%3D&reserved=0 > > > > The NWL driver needs to be adjusted depending on whether we hook into > > imx-display-subsystem or not (and then likely moved to the right > > subdir). Can we somehow get this moving in sync (even in a non public > > tree if necessary). > > I guess we could do that as well. I'll start adjusting the driver and > take it out of imx-drm, as suggested by Lucas and Daniel. I'll use your > MIPI patches to test with. Cool. this will likely mean turning the NWL part into a bridge which I can look into. We want that when interfacing with mxsfb anyway. Cheers, -- Guido
Hi Laurentiu, On Tue, May 28, 2019 at 10:10:11AM +0000, Laurentiu Palcu wrote: > Hi Guido, > > On Tue, May 28, 2019 at 11:33:00AM +0200, Guido Günther wrote: > > Caution: EXT Email > > > > Hi Laurentiu, > > On Tue, May 28, 2019 at 07:03:54AM +0000, Laurentiu Palcu wrote: > > > Hi Shawn, Lucas, > > > > > > On Tue, May 28, 2019 at 09:38:02AM +0800, Shawn Guo wrote: > > > > Caution: EXT Email > > > > > > > > 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? > > > > > > We're already working on clearing up the DCSS code and preparing it for > > > upstreaming. It should be done in the following weeks. The reason we've > > > been delaying this is because neither HDMI nor MIPI support was present > > > and, until these are upstream, testing DCSS would be quite impossible. > > > > MIPI support is here: > > > > mixel: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F58817%2F&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839828366&sdata=VZvvhe2WkMVCSOEw5oZDJfy7rsqF7YaEirrkLFC8Icw%3D&reserved=0 > > nwl: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F57686%2F&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=Q4s6OZq1KElktjSXRd2lKBMdg1yPJsWGm8UrSPqqTiE%3D&reserved=0 > > > > The NWL driver needs to be adjusted depending on whether we hook into > > imx-display-subsystem or not (and then likely moved to the right > > subdir). Can we somehow get this moving in sync (even in a non public > > tree if necessary). > > I guess we could do that as well. I'll start adjusting the driver and > take it out of imx-drm, as suggested by Lucas and Daniel. I'll use your > MIPI patches to test with. Is there anything I could test against v1 of the imx-nwl driver to make sure that works as well: https://patchwork.freedesktop.org/series/64185/ Is your focus more towards DSI or HDMI / DP for inital submission? Cheers, -- Guido > > thanks, > laurentiu > > > Cheers, > > -- Guido > > > > > > > > > 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? > > > > > > I'd be interested to hear about this as well. > > > > > > > > > > > > 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. > > > > > > I also agree on this. DCSS core code will probably be moved inside the > > > same directory: drivers/gpu/drm/imx/dcss. > > > > > > Thanks, > > > laurentiu > > > > > > > > > > > Shawn > > > > _______________________________________________ > > > > 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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=6I8MCXrt3y4nX20SnpfoSwEZkg%2B1zP3AFLGHUNaI%2Fls%3D&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&data=02%7C01%7Claurentiu.palcu%40nxp.com%7C9924c4628d5f4f98d8a808d6e34f7b26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636946327839838359&sdata=6I8MCXrt3y4nX20SnpfoSwEZkg%2B1zP3AFLGHUNaI%2Fls%3D&reserved=0