| Message ID | 20220829184031.1863663-1-jagan@amarulasolutions.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: bridge: Add Samsung MIPI DSIM bridge | expand |
On 29.08.2022 20:40, Jagan Teki wrote: > Samsung MIPI DSIM controller is common DSI IP that can be used in various > SoCs like Exynos, i.MX8M Mini/Nano. > > In order to access this DSI controller between various platform SoCs, > the ideal way to incorporate this in the drm stack is via the drm bridge > driver. > > This patch is trying to differentiate platform-specific and bridge driver > code and keep maintaining the exynos_drm_dsi.c code as platform-specific > glue code and samsung-dsim.c as a common bridge driver code. > > - Exynos specific glue code is exynos specific te_irq, host_attach, and > detach code along with conventional component_ops. > > - Samsung DSIM is a bridge driver which is common across all platforms and > the respective platform-specific glue will initialize at the end of the > probe. The platform-specific operations and other glue calls will invoke > on associate code areas. > > v4: > * include Inki Dae in MAINTAINERS > * remove dsi_driver probe in exynos_drm_drv to support multi-arch build This breaks Exynos DRM completely as the Exynos DRM driver is not able to wait until the DSI driver is probed and registered as component. I will show how to rework this the way it is done in drivers/gpu/drm/exynos/exynos_dp.c and drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... > v3: > * restore gpio related fixes > * restore proper bridge chain > * rework initialization issue > * fix header includes in proper way > > v2: > * fixed exynos dsi driver conversion (Marek Szyprowski) > * updated commit message > * updated MAINTAINERS file > > v1: > * don't maintain component_ops in bridge driver > * don't maintain platform glue code in bridge driver > * add platform-specific glue code and make a common bridge > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > MAINTAINERS | 9 + > drivers/gpu/drm/bridge/Kconfig | 12 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/samsung-dsim.c | 1686 ++++++++++++++++++++++ > drivers/gpu/drm/exynos/Kconfig | 1 + > drivers/gpu/drm/exynos/exynos_drm_drv.c | 3 - > drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 - > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1715 +---------------------- > include/drm/bridge/samsung-dsim.h | 99 ++ > 9 files changed, 1868 insertions(+), 1659 deletions(-) > create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c > create mode 100644 include/drm/bridge/samsung-dsim.h > > ... Best regards
Hi All, On 02.09.2022 12:47, Marek Szyprowski wrote: > On 29.08.2022 20:40, Jagan Teki wrote: >> Samsung MIPI DSIM controller is common DSI IP that can be used in >> various >> SoCs like Exynos, i.MX8M Mini/Nano. >> >> In order to access this DSI controller between various platform SoCs, >> the ideal way to incorporate this in the drm stack is via the drm bridge >> driver. >> >> This patch is trying to differentiate platform-specific and bridge >> driver >> code and keep maintaining the exynos_drm_dsi.c code as platform-specific >> glue code and samsung-dsim.c as a common bridge driver code. >> >> - Exynos specific glue code is exynos specific te_irq, host_attach, and >> detach code along with conventional component_ops. >> >> - Samsung DSIM is a bridge driver which is common across all >> platforms and >> the respective platform-specific glue will initialize at the end >> of the >> probe. The platform-specific operations and other glue calls will >> invoke >> on associate code areas. >> >> v4: >> * include Inki Dae in MAINTAINERS >> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > > This breaks Exynos DRM completely as the Exynos DRM driver is not able > to wait until the DSI driver is probed and registered as component. > > I will show how to rework this the way it is done in > drivers/gpu/drm/exynos/exynos_dp.c and > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... I've finally had some time to implement such approach, see https://github.com/mszyprow/linux/tree/v6.0-dsi-v4-reworked If you want me to send the patches against your v4 patchset, let me know, but imho my changes are much more readable after squashing to the original patches. Now the driver is fully multi-arch safe and ready for further extensions. I've removed the weak functions, reworked the way the plat_data is used (dropped the patch related to it) and restored exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel free to resend the above as v5 after testing on your hardware. At least it properly works now on all Exynos boards I have, both compiled into the kernel or as modules. > >> v3: >> * restore gpio related fixes >> * restore proper bridge chain >> * rework initialization issue >> * fix header includes in proper way >> >> v2: >> * fixed exynos dsi driver conversion (Marek Szyprowski) >> * updated commit message >> * updated MAINTAINERS file >> >> v1: >> * don't maintain component_ops in bridge driver >> * don't maintain platform glue code in bridge driver >> * add platform-specific glue code and make a common bridge >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >> --- >> MAINTAINERS | 9 + >> drivers/gpu/drm/bridge/Kconfig | 12 + >> drivers/gpu/drm/bridge/Makefile | 1 + >> drivers/gpu/drm/bridge/samsung-dsim.c | 1686 ++++++++++++++++++++++ >> drivers/gpu/drm/exynos/Kconfig | 1 + >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 3 - >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 - >> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1715 +---------------------- >> include/drm/bridge/samsung-dsim.h | 99 ++ >> 9 files changed, 1868 insertions(+), 1659 deletions(-) >> create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c >> create mode 100644 include/drm/bridge/samsung-dsim.h >> >> ... > > Best regards Best regards
On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi All, > > On 02.09.2022 12:47, Marek Szyprowski wrote: > > On 29.08.2022 20:40, Jagan Teki wrote: > >> Samsung MIPI DSIM controller is common DSI IP that can be used in > >> various > >> SoCs like Exynos, i.MX8M Mini/Nano. > >> > >> In order to access this DSI controller between various platform SoCs, > >> the ideal way to incorporate this in the drm stack is via the drm bridge > >> driver. > >> > >> This patch is trying to differentiate platform-specific and bridge > >> driver > >> code and keep maintaining the exynos_drm_dsi.c code as platform-specific > >> glue code and samsung-dsim.c as a common bridge driver code. > >> > >> - Exynos specific glue code is exynos specific te_irq, host_attach, and > >> detach code along with conventional component_ops. > >> > >> - Samsung DSIM is a bridge driver which is common across all > >> platforms and > >> the respective platform-specific glue will initialize at the end > >> of the > >> probe. The platform-specific operations and other glue calls will > >> invoke > >> on associate code areas. > >> > >> v4: > >> * include Inki Dae in MAINTAINERS > >> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > > > > This breaks Exynos DRM completely as the Exynos DRM driver is not able > > to wait until the DSI driver is probed and registered as component. > > > > I will show how to rework this the way it is done in > > drivers/gpu/drm/exynos/exynos_dp.c and > > drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... > > I've finally had some time to implement such approach, see > https://github.com/mszyprow/linux/tree/v6.0-dsi-v4-reworked > > If you want me to send the patches against your v4 patchset, let me > know, but imho my changes are much more readable after squashing to the > original patches. > > Now the driver is fully multi-arch safe and ready for further > extensions. I've removed the weak functions, reworked the way the > plat_data is used (dropped the patch related to it) and restored > exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel > free to resend the above as v5 after testing on your hardware. At least > it properly works now on all Exynos boards I have, both compiled into > the kernel or as modules. Thanks. I've seen the repo added on top of Dave patches - does it mean these depends on Dave changes as well? Jagan.
Hi Jagan, On 06.09.2022 21:07, Jagan Teki wrote: > On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> On 02.09.2022 12:47, Marek Szyprowski wrote: >>> On 29.08.2022 20:40, Jagan Teki wrote: >>>> Samsung MIPI DSIM controller is common DSI IP that can be used in >>>> various >>>> SoCs like Exynos, i.MX8M Mini/Nano. >>>> >>>> In order to access this DSI controller between various platform SoCs, >>>> the ideal way to incorporate this in the drm stack is via the drm bridge >>>> driver. >>>> >>>> This patch is trying to differentiate platform-specific and bridge >>>> driver >>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific >>>> glue code and samsung-dsim.c as a common bridge driver code. >>>> >>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and >>>> detach code along with conventional component_ops. >>>> >>>> - Samsung DSIM is a bridge driver which is common across all >>>> platforms and >>>> the respective platform-specific glue will initialize at the end >>>> of the >>>> probe. The platform-specific operations and other glue calls will >>>> invoke >>>> on associate code areas. >>>> >>>> v4: >>>> * include Inki Dae in MAINTAINERS >>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build >>> This breaks Exynos DRM completely as the Exynos DRM driver is not able >>> to wait until the DSI driver is probed and registered as component. >>> >>> I will show how to rework this the way it is done in >>> drivers/gpu/drm/exynos/exynos_dp.c and >>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... >> I've finally had some time to implement such approach, see >> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked >> >> If you want me to send the patches against your v4 patchset, let me >> know, but imho my changes are much more readable after squashing to the >> original patches. >> >> Now the driver is fully multi-arch safe and ready for further >> extensions. I've removed the weak functions, reworked the way the >> plat_data is used (dropped the patch related to it) and restored >> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel >> free to resend the above as v5 after testing on your hardware. At least >> it properly works now on all Exynos boards I have, both compiled into >> the kernel or as modules. > Thanks. I've seen the repo added on top of Dave patches - does it mean > these depends on Dave changes as well? Yes and no. My rework doesn't change anything with this dependency. It comes from my patch "drm: exynos: dsi: Restore proper bridge chain order" already included in your series (patch #1). Without it exynos-dsi driver hacks the list of bridges to ensure the order of pre_enable calls needed for proper operation. This works somehow with DSI panels on my test systems, but it has been reported that it doesn't work with a bit more complex display pipelines. Only that patch depends on the Dave's patches. If you remove it, you would need to adjust the code in the exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be better to keep it and merge Dave's patches together with dsi changes, as they are the first real client of it. Best regards
Hi Jagan, Marek, Am 07.09.22 um 12:04 schrieb Marek Szyprowski: > Hi Jagan, > > On 06.09.2022 21:07, Jagan Teki wrote: >> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski >> <m.szyprowski@samsung.com> wrote: >>> On 02.09.2022 12:47, Marek Szyprowski wrote: >>>> On 29.08.2022 20:40, Jagan Teki wrote: >>>>> Samsung MIPI DSIM controller is common DSI IP that can be used in >>>>> various >>>>> SoCs like Exynos, i.MX8M Mini/Nano. >>>>> >>>>> In order to access this DSI controller between various platform SoCs, >>>>> the ideal way to incorporate this in the drm stack is via the drm bridge >>>>> driver. >>>>> >>>>> This patch is trying to differentiate platform-specific and bridge >>>>> driver >>>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific >>>>> glue code and samsung-dsim.c as a common bridge driver code. >>>>> >>>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and >>>>> detach code along with conventional component_ops. >>>>> >>>>> - Samsung DSIM is a bridge driver which is common across all >>>>> platforms and >>>>> the respective platform-specific glue will initialize at the end >>>>> of the >>>>> probe. The platform-specific operations and other glue calls will >>>>> invoke >>>>> on associate code areas. >>>>> >>>>> v4: >>>>> * include Inki Dae in MAINTAINERS >>>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build >>>> This breaks Exynos DRM completely as the Exynos DRM driver is not able >>>> to wait until the DSI driver is probed and registered as component. >>>> >>>> I will show how to rework this the way it is done in >>>> drivers/gpu/drm/exynos/exynos_dp.c and >>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... >>> I've finally had some time to implement such approach, see >>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Fv1%2Furl%3Fk%3Dc5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375%26q%3D1%26e%3D489b94d4-84fb-408e-b679-a8d27acf2930%26u%3Dhttps%253A%252F%252Fgithub.com%252Fmszyprow%252Flinux%252Ftree%252Fv6.0-dsi-v4-reworked&data=05%7C01%7Cfrieder.schrempf%40kontron.de%7C8ed9bd90703b48c9d43708da90b86876%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637981418959345104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=vLwOMBbdNo1C5x%2BESY3SU%2FwaYKBmAgIyafLOLmd3BlM%3D&reserved=0 >>> >>> If you want me to send the patches against your v4 patchset, let me >>> know, but imho my changes are much more readable after squashing to the >>> original patches. >>> >>> Now the driver is fully multi-arch safe and ready for further >>> extensions. I've removed the weak functions, reworked the way the >>> plat_data is used (dropped the patch related to it) and restored >>> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel >>> free to resend the above as v5 after testing on your hardware. At least >>> it properly works now on all Exynos boards I have, both compiled into >>> the kernel or as modules. >> Thanks. I've seen the repo added on top of Dave patches - does it mean >> these depends on Dave changes as well? > > Yes and no. My rework doesn't change anything with this dependency. It > comes from my patch "drm: exynos: dsi: Restore proper bridge chain > order" already included in your series (patch #1). Without it exynos-dsi > driver hacks the list of bridges to ensure the order of pre_enable calls > needed for proper operation. This works somehow with DSI panels on my > test systems, but it has been reported that it doesn't work with a bit > more complex display pipelines. Only that patch depends on the Dave's > patches. If you remove it, you would need to adjust the code in the > exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be > better to keep it and merge Dave's patches together with dsi changes, as > they are the first real client of it. I tried to test Jagan's v4 on our Kontron BL i.MX8MM with SN65DSI84 LVDS bridge and it didn't work due to driver dependency issues. Instead trying Marek's branch rebased to 6.0-rc4 and including the necessary DT changes from Jagan's v4 looks good and produces an image on the display. Only one thing caused problems: In Jagan's patch for the LCDIF node [1] there's a typo (missing 's') in the assigned-clock-rates property. Without fixing this the display doesn't work. Jagan, can you come up with a v5 including Marek's fixes? And if you like, I think you could also start sending the DT changes for i.MX8MM, so we can start reviewing/testing them? Thanks Frieder [1]: https://github.com/openedev/kernel/commit/89a6252cec47f3593d4f2b7ea3132f4745c4fdb3
On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Jagan, > > On 06.09.2022 21:07, Jagan Teki wrote: > > On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski > > <m.szyprowski@samsung.com> wrote: > >> On 02.09.2022 12:47, Marek Szyprowski wrote: > >>> On 29.08.2022 20:40, Jagan Teki wrote: > >>>> Samsung MIPI DSIM controller is common DSI IP that can be used in > >>>> various > >>>> SoCs like Exynos, i.MX8M Mini/Nano. > >>>> > >>>> In order to access this DSI controller between various platform SoCs, > >>>> the ideal way to incorporate this in the drm stack is via the drm bridge > >>>> driver. > >>>> > >>>> This patch is trying to differentiate platform-specific and bridge > >>>> driver > >>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific > >>>> glue code and samsung-dsim.c as a common bridge driver code. > >>>> > >>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and > >>>> detach code along with conventional component_ops. > >>>> > >>>> - Samsung DSIM is a bridge driver which is common across all > >>>> platforms and > >>>> the respective platform-specific glue will initialize at the end > >>>> of the > >>>> probe. The platform-specific operations and other glue calls will > >>>> invoke > >>>> on associate code areas. > >>>> > >>>> v4: > >>>> * include Inki Dae in MAINTAINERS > >>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > >>> This breaks Exynos DRM completely as the Exynos DRM driver is not able > >>> to wait until the DSI driver is probed and registered as component. > >>> > >>> I will show how to rework this the way it is done in > >>> drivers/gpu/drm/exynos/exynos_dp.c and > >>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... > >> I've finally had some time to implement such approach, see > >> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked > >> > >> If you want me to send the patches against your v4 patchset, let me > >> know, but imho my changes are much more readable after squashing to the > >> original patches. > >> > >> Now the driver is fully multi-arch safe and ready for further > >> extensions. I've removed the weak functions, reworked the way the > >> plat_data is used (dropped the patch related to it) and restored > >> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel > >> free to resend the above as v5 after testing on your hardware. At least > >> it properly works now on all Exynos boards I have, both compiled into > >> the kernel or as modules. > > Thanks. I've seen the repo added on top of Dave patches - does it mean > > these depends on Dave changes as well? > > Yes and no. My rework doesn't change anything with this dependency. It > comes from my patch "drm: exynos: dsi: Restore proper bridge chain > order" already included in your series (patch #1). Without it exynos-dsi > driver hacks the list of bridges to ensure the order of pre_enable calls > needed for proper operation. This works somehow with DSI panels on my > test systems, but it has been reported that it doesn't work with a bit > more complex display pipelines. Only that patch depends on the Dave's > patches. If you remove it, you would need to adjust the code in the > exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be > better to keep it and merge Dave's patches together with dsi changes, as > they are the first real client of it. I think the Dave patches especially "drm/bridge: Introduce pre_enable_upstream_first to alter bridge init order" seems not 100% relevant to this series as they affect bridge chain call flow globally. Having a separate series for that makes sense to me. I'm sending v5 by excluding those parts. Jagan.
Hi Jagan, On 13.09.2022 19:29, Jagan Teki wrote: > On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> On 06.09.2022 21:07, Jagan Teki wrote: >>> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski >>> <m.szyprowski@samsung.com> wrote: >>>> On 02.09.2022 12:47, Marek Szyprowski wrote: >>>>> On 29.08.2022 20:40, Jagan Teki wrote: >>>>>> Samsung MIPI DSIM controller is common DSI IP that can be used in >>>>>> various >>>>>> SoCs like Exynos, i.MX8M Mini/Nano. >>>>>> >>>>>> In order to access this DSI controller between various platform SoCs, >>>>>> the ideal way to incorporate this in the drm stack is via the drm bridge >>>>>> driver. >>>>>> >>>>>> This patch is trying to differentiate platform-specific and bridge >>>>>> driver >>>>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific >>>>>> glue code and samsung-dsim.c as a common bridge driver code. >>>>>> >>>>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and >>>>>> detach code along with conventional component_ops. >>>>>> >>>>>> - Samsung DSIM is a bridge driver which is common across all >>>>>> platforms and >>>>>> the respective platform-specific glue will initialize at the end >>>>>> of the >>>>>> probe. The platform-specific operations and other glue calls will >>>>>> invoke >>>>>> on associate code areas. >>>>>> >>>>>> v4: >>>>>> * include Inki Dae in MAINTAINERS >>>>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build >>>>> This breaks Exynos DRM completely as the Exynos DRM driver is not able >>>>> to wait until the DSI driver is probed and registered as component. >>>>> >>>>> I will show how to rework this the way it is done in >>>>> drivers/gpu/drm/exynos/exynos_dp.c and >>>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... >>>> I've finally had some time to implement such approach, see >>>> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked >>>> >>>> If you want me to send the patches against your v4 patchset, let me >>>> know, but imho my changes are much more readable after squashing to the >>>> original patches. >>>> >>>> Now the driver is fully multi-arch safe and ready for further >>>> extensions. I've removed the weak functions, reworked the way the >>>> plat_data is used (dropped the patch related to it) and restored >>>> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel >>>> free to resend the above as v5 after testing on your hardware. At least >>>> it properly works now on all Exynos boards I have, both compiled into >>>> the kernel or as modules. >>> Thanks. I've seen the repo added on top of Dave patches - does it mean >>> these depends on Dave changes as well? >> Yes and no. My rework doesn't change anything with this dependency. It >> comes from my patch "drm: exynos: dsi: Restore proper bridge chain >> order" already included in your series (patch #1). Without it exynos-dsi >> driver hacks the list of bridges to ensure the order of pre_enable calls >> needed for proper operation. This works somehow with DSI panels on my >> test systems, but it has been reported that it doesn't work with a bit >> more complex display pipelines. Only that patch depends on the Dave's >> patches. If you remove it, you would need to adjust the code in the >> exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be >> better to keep it and merge Dave's patches together with dsi changes, as >> they are the first real client of it. > I think the Dave patches especially "drm/bridge: Introduce > pre_enable_upstream_first to alter bridge init order" seems not 100% > relevant to this series as they affect bridge chain call flow > globally. Having a separate series for that makes sense to me. I'm > sending v5 by excluding those parts. If so then drop the "drm: exynos: dsi: Restore proper bridge chain order" patch and adjust code respectively in samsung-dsim.c. Without the Dave's patches, that one doesn't make sense. Best regards
On Wed, Sep 14, 2022 at 2:51 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Jagan, > > On 13.09.2022 19:29, Jagan Teki wrote: > > On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski > > <m.szyprowski@samsung.com> wrote: > >> On 06.09.2022 21:07, Jagan Teki wrote: > >>> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski > >>> <m.szyprowski@samsung.com> wrote: > >>>> On 02.09.2022 12:47, Marek Szyprowski wrote: > >>>>> On 29.08.2022 20:40, Jagan Teki wrote: > >>>>>> Samsung MIPI DSIM controller is common DSI IP that can be used in > >>>>>> various > >>>>>> SoCs like Exynos, i.MX8M Mini/Nano. > >>>>>> > >>>>>> In order to access this DSI controller between various platform SoCs, > >>>>>> the ideal way to incorporate this in the drm stack is via the drm bridge > >>>>>> driver. > >>>>>> > >>>>>> This patch is trying to differentiate platform-specific and bridge > >>>>>> driver > >>>>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific > >>>>>> glue code and samsung-dsim.c as a common bridge driver code. > >>>>>> > >>>>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and > >>>>>> detach code along with conventional component_ops. > >>>>>> > >>>>>> - Samsung DSIM is a bridge driver which is common across all > >>>>>> platforms and > >>>>>> the respective platform-specific glue will initialize at the end > >>>>>> of the > >>>>>> probe. The platform-specific operations and other glue calls will > >>>>>> invoke > >>>>>> on associate code areas. > >>>>>> > >>>>>> v4: > >>>>>> * include Inki Dae in MAINTAINERS > >>>>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > >>>>> This breaks Exynos DRM completely as the Exynos DRM driver is not able > >>>>> to wait until the DSI driver is probed and registered as component. > >>>>> > >>>>> I will show how to rework this the way it is done in > >>>>> drivers/gpu/drm/exynos/exynos_dp.c and > >>>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... > >>>> I've finally had some time to implement such approach, see > >>>> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked > >>>> > >>>> If you want me to send the patches against your v4 patchset, let me > >>>> know, but imho my changes are much more readable after squashing to the > >>>> original patches. > >>>> > >>>> Now the driver is fully multi-arch safe and ready for further > >>>> extensions. I've removed the weak functions, reworked the way the > >>>> plat_data is used (dropped the patch related to it) and restored > >>>> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel > >>>> free to resend the above as v5 after testing on your hardware. At least > >>>> it properly works now on all Exynos boards I have, both compiled into > >>>> the kernel or as modules. > >>> Thanks. I've seen the repo added on top of Dave patches - does it mean > >>> these depends on Dave changes as well? > >> Yes and no. My rework doesn't change anything with this dependency. It > >> comes from my patch "drm: exynos: dsi: Restore proper bridge chain > >> order" already included in your series (patch #1). Without it exynos-dsi > >> driver hacks the list of bridges to ensure the order of pre_enable calls > >> needed for proper operation. This works somehow with DSI panels on my > >> test systems, but it has been reported that it doesn't work with a bit > >> more complex display pipelines. Only that patch depends on the Dave's > >> patches. If you remove it, you would need to adjust the code in the > >> exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be > >> better to keep it and merge Dave's patches together with dsi changes, as > >> they are the first real client of it. > > I think the Dave patches especially "drm/bridge: Introduce > > pre_enable_upstream_first to alter bridge init order" seems not 100% > > relevant to this series as they affect bridge chain call flow > > globally. Having a separate series for that makes sense to me. I'm > > sending v5 by excluding those parts. > > If so then drop the "drm: exynos: dsi: Restore proper bridge chain > order" patch and adjust code respectively in samsung-dsim.c. Without the > Dave's patches, that one doesn't make sense. Doesn't it break Exynos? Jagan.
Hi Jagan, On 14.09.2022 11:39, Jagan Teki wrote: > On Wed, Sep 14, 2022 at 2:51 PM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> On 13.09.2022 19:29, Jagan Teki wrote: >>> On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski >>> <m.szyprowski@samsung.com> wrote: >>>> On 06.09.2022 21:07, Jagan Teki wrote: >>>>> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski >>>>> <m.szyprowski@samsung.com> wrote: >>>>>> On 02.09.2022 12:47, Marek Szyprowski wrote: >>>>>>> On 29.08.2022 20:40, Jagan Teki wrote: >>>>>>>> Samsung MIPI DSIM controller is common DSI IP that can be used in >>>>>>>> various >>>>>>>> SoCs like Exynos, i.MX8M Mini/Nano. >>>>>>>> >>>>>>>> In order to access this DSI controller between various platform SoCs, >>>>>>>> the ideal way to incorporate this in the drm stack is via the drm bridge >>>>>>>> driver. >>>>>>>> >>>>>>>> This patch is trying to differentiate platform-specific and bridge >>>>>>>> driver >>>>>>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific >>>>>>>> glue code and samsung-dsim.c as a common bridge driver code. >>>>>>>> >>>>>>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and >>>>>>>> detach code along with conventional component_ops. >>>>>>>> >>>>>>>> - Samsung DSIM is a bridge driver which is common across all >>>>>>>> platforms and >>>>>>>> the respective platform-specific glue will initialize at the end >>>>>>>> of the >>>>>>>> probe. The platform-specific operations and other glue calls will >>>>>>>> invoke >>>>>>>> on associate code areas. >>>>>>>> >>>>>>>> v4: >>>>>>>> * include Inki Dae in MAINTAINERS >>>>>>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build >>>>>>> This breaks Exynos DRM completely as the Exynos DRM driver is not able >>>>>>> to wait until the DSI driver is probed and registered as component. >>>>>>> >>>>>>> I will show how to rework this the way it is done in >>>>>>> drivers/gpu/drm/exynos/exynos_dp.c and >>>>>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... >>>>>> I've finally had some time to implement such approach, see >>>>>> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked >>>>>> >>>>>> If you want me to send the patches against your v4 patchset, let me >>>>>> know, but imho my changes are much more readable after squashing to the >>>>>> original patches. >>>>>> >>>>>> Now the driver is fully multi-arch safe and ready for further >>>>>> extensions. I've removed the weak functions, reworked the way the >>>>>> plat_data is used (dropped the patch related to it) and restored >>>>>> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel >>>>>> free to resend the above as v5 after testing on your hardware. At least >>>>>> it properly works now on all Exynos boards I have, both compiled into >>>>>> the kernel or as modules. >>>>> Thanks. I've seen the repo added on top of Dave patches - does it mean >>>>> these depends on Dave changes as well? >>>> Yes and no. My rework doesn't change anything with this dependency. It >>>> comes from my patch "drm: exynos: dsi: Restore proper bridge chain >>>> order" already included in your series (patch #1). Without it exynos-dsi >>>> driver hacks the list of bridges to ensure the order of pre_enable calls >>>> needed for proper operation. This works somehow with DSI panels on my >>>> test systems, but it has been reported that it doesn't work with a bit >>>> more complex display pipelines. Only that patch depends on the Dave's >>>> patches. If you remove it, you would need to adjust the code in the >>>> exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be >>>> better to keep it and merge Dave's patches together with dsi changes, as >>>> they are the first real client of it. >>> I think the Dave patches especially "drm/bridge: Introduce >>> pre_enable_upstream_first to alter bridge init order" seems not 100% >>> relevant to this series as they affect bridge chain call flow >>> globally. Having a separate series for that makes sense to me. I'm >>> sending v5 by excluding those parts. >> If so then drop the "drm: exynos: dsi: Restore proper bridge chain >> order" patch and adjust code respectively in samsung-dsim.c. Without the >> Dave's patches, that one doesn't make sense. > Doesn't it break Exynos? No it won't. Lack of the "drm: exynos: dsi: Restore proper bridge chain order" patch doesn't change much against the current state of the driver. Here is my rework of your v4 patchset without the mentioned patch and Dave's patches: https://github.com/mszyprow/linux/tree/v6.0-dsi-v4-reworked-minimal Best regards
On Fri, Sep 16, 2022 at 1:58 PM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Jagan, > > On 14.09.2022 11:39, Jagan Teki wrote: > > On Wed, Sep 14, 2022 at 2:51 PM Marek Szyprowski > > <m.szyprowski@samsung.com> wrote: > >> On 13.09.2022 19:29, Jagan Teki wrote: > >>> On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski > >>> <m.szyprowski@samsung.com> wrote: > >>>> On 06.09.2022 21:07, Jagan Teki wrote: > >>>>> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski > >>>>> <m.szyprowski@samsung.com> wrote: > >>>>>> On 02.09.2022 12:47, Marek Szyprowski wrote: > >>>>>>> On 29.08.2022 20:40, Jagan Teki wrote: > >>>>>>>> Samsung MIPI DSIM controller is common DSI IP that can be used in > >>>>>>>> various > >>>>>>>> SoCs like Exynos, i.MX8M Mini/Nano. > >>>>>>>> > >>>>>>>> In order to access this DSI controller between various platform SoCs, > >>>>>>>> the ideal way to incorporate this in the drm stack is via the drm bridge > >>>>>>>> driver. > >>>>>>>> > >>>>>>>> This patch is trying to differentiate platform-specific and bridge > >>>>>>>> driver > >>>>>>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific > >>>>>>>> glue code and samsung-dsim.c as a common bridge driver code. > >>>>>>>> > >>>>>>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and > >>>>>>>> detach code along with conventional component_ops. > >>>>>>>> > >>>>>>>> - Samsung DSIM is a bridge driver which is common across all > >>>>>>>> platforms and > >>>>>>>> the respective platform-specific glue will initialize at the end > >>>>>>>> of the > >>>>>>>> probe. The platform-specific operations and other glue calls will > >>>>>>>> invoke > >>>>>>>> on associate code areas. > >>>>>>>> > >>>>>>>> v4: > >>>>>>>> * include Inki Dae in MAINTAINERS > >>>>>>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build > >>>>>>> This breaks Exynos DRM completely as the Exynos DRM driver is not able > >>>>>>> to wait until the DSI driver is probed and registered as component. > >>>>>>> > >>>>>>> I will show how to rework this the way it is done in > >>>>>>> drivers/gpu/drm/exynos/exynos_dp.c and > >>>>>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... > >>>>>> I've finally had some time to implement such approach, see > >>>>>> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked > >>>>>> > >>>>>> If you want me to send the patches against your v4 patchset, let me > >>>>>> know, but imho my changes are much more readable after squashing to the > >>>>>> original patches. > >>>>>> > >>>>>> Now the driver is fully multi-arch safe and ready for further > >>>>>> extensions. I've removed the weak functions, reworked the way the > >>>>>> plat_data is used (dropped the patch related to it) and restored > >>>>>> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel > >>>>>> free to resend the above as v5 after testing on your hardware. At least > >>>>>> it properly works now on all Exynos boards I have, both compiled into > >>>>>> the kernel or as modules. > >>>>> Thanks. I've seen the repo added on top of Dave patches - does it mean > >>>>> these depends on Dave changes as well? > >>>> Yes and no. My rework doesn't change anything with this dependency. It > >>>> comes from my patch "drm: exynos: dsi: Restore proper bridge chain > >>>> order" already included in your series (patch #1). Without it exynos-dsi > >>>> driver hacks the list of bridges to ensure the order of pre_enable calls > >>>> needed for proper operation. This works somehow with DSI panels on my > >>>> test systems, but it has been reported that it doesn't work with a bit > >>>> more complex display pipelines. Only that patch depends on the Dave's > >>>> patches. If you remove it, you would need to adjust the code in the > >>>> exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be > >>>> better to keep it and merge Dave's patches together with dsi changes, as > >>>> they are the first real client of it. > >>> I think the Dave patches especially "drm/bridge: Introduce > >>> pre_enable_upstream_first to alter bridge init order" seems not 100% > >>> relevant to this series as they affect bridge chain call flow > >>> globally. Having a separate series for that makes sense to me. I'm > >>> sending v5 by excluding those parts. > >> If so then drop the "drm: exynos: dsi: Restore proper bridge chain > >> order" patch and adjust code respectively in samsung-dsim.c. Without the > >> Dave's patches, that one doesn't make sense. > > Doesn't it break Exynos? > > No it won't. Lack of the "drm: exynos: dsi: Restore proper bridge chain > order" patch doesn't change much against the current state of the driver. > > Here is my rework of your v4 patchset without the mentioned patch and > Dave's patches: > > https://github.com/mszyprow/linux/tree/v6.0-dsi-v4-reworked-minimal We have one problem with getting bus format from previous bridge if we pass NULL in bridge_func.attach() https://github.com/mszyprow/linux/commit/0fa57e33b3bf866efc4c17ab20eec28d6e07b3e9#diff-3fe873f1ada5f1dfcf2a50ac114bdab3ea7b026d12278648ca40809d3fa1a331R1321 Booting the video as it assigns default bus format if the previous bus format is unknown. [ 1.635984] samsung-dsim 32e10000.dsi: [drm:samsung_dsim_host_attach] Attached sn65dsi83 device [ 1.648067] [drm] Initialized mxsfb-drm 1.0.0 20160824 for 32e00000.lcdif on minor 0 [ 1.658726] mmc0: SDHCI controller on 30b40000.mmc [30b40000.mmc] using ADMA [ 1.681893] sn65dsi83 3-002c: Unsupported LVDS bus format 0x100a, please check output bridge driver. Falling back to SPWG24. Does passing the bridge to drm_bridge_attach is working on your platform? return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, flags); Thanks, Jagan.
Hi Jagan, On 16.09.2022 12:21, Jagan Teki wrote: > On Fri, Sep 16, 2022 at 1:58 PM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: >> On 14.09.2022 11:39, Jagan Teki wrote: >>> On Wed, Sep 14, 2022 at 2:51 PM Marek Szyprowski >>> <m.szyprowski@samsung.com> wrote: >>>> On 13.09.2022 19:29, Jagan Teki wrote: >>>>> On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski >>>>> <m.szyprowski@samsung.com> wrote: >>>>>> On 06.09.2022 21:07, Jagan Teki wrote: >>>>>>> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski >>>>>>> <m.szyprowski@samsung.com> wrote: >>>>>>>> On 02.09.2022 12:47, Marek Szyprowski wrote: >>>>>>>>> On 29.08.2022 20:40, Jagan Teki wrote: >>>>>>>>>> Samsung MIPI DSIM controller is common DSI IP that can be used in >>>>>>>>>> various >>>>>>>>>> SoCs like Exynos, i.MX8M Mini/Nano. >>>>>>>>>> >>>>>>>>>> In order to access this DSI controller between various platform SoCs, >>>>>>>>>> the ideal way to incorporate this in the drm stack is via the drm bridge >>>>>>>>>> driver. >>>>>>>>>> >>>>>>>>>> This patch is trying to differentiate platform-specific and bridge >>>>>>>>>> driver >>>>>>>>>> code and keep maintaining the exynos_drm_dsi.c code as platform-specific >>>>>>>>>> glue code and samsung-dsim.c as a common bridge driver code. >>>>>>>>>> >>>>>>>>>> - Exynos specific glue code is exynos specific te_irq, host_attach, and >>>>>>>>>> detach code along with conventional component_ops. >>>>>>>>>> >>>>>>>>>> - Samsung DSIM is a bridge driver which is common across all >>>>>>>>>> platforms and >>>>>>>>>> the respective platform-specific glue will initialize at the end >>>>>>>>>> of the >>>>>>>>>> probe. The platform-specific operations and other glue calls will >>>>>>>>>> invoke >>>>>>>>>> on associate code areas. >>>>>>>>>> >>>>>>>>>> v4: >>>>>>>>>> * include Inki Dae in MAINTAINERS >>>>>>>>>> * remove dsi_driver probe in exynos_drm_drv to support multi-arch build >>>>>>>>> This breaks Exynos DRM completely as the Exynos DRM driver is not able >>>>>>>>> to wait until the DSI driver is probed and registered as component. >>>>>>>>> >>>>>>>>> I will show how to rework this the way it is done in >>>>>>>>> drivers/gpu/drm/exynos/exynos_dp.c and >>>>>>>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c soon... >>>>>>>> I've finally had some time to implement such approach, see >>>>>>>> https://protect2.fireeye.com/v1/url?k=c5d024d9-a4ab8e4e-c5d1af96-74fe4860001d-625a8324a9797375&q=1&e=489b94d4-84fb-408e-b679-a8d27acf2930&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked >>>>>>>> >>>>>>>> If you want me to send the patches against your v4 patchset, let me >>>>>>>> know, but imho my changes are much more readable after squashing to the >>>>>>>> original patches. >>>>>>>> >>>>>>>> Now the driver is fully multi-arch safe and ready for further >>>>>>>> extensions. I've removed the weak functions, reworked the way the >>>>>>>> plat_data is used (dropped the patch related to it) and restored >>>>>>>> exynos-dsi driver as a part of the Exynos DRM drivers/subsystem. Feel >>>>>>>> free to resend the above as v5 after testing on your hardware. At least >>>>>>>> it properly works now on all Exynos boards I have, both compiled into >>>>>>>> the kernel or as modules. >>>>>>> Thanks. I've seen the repo added on top of Dave patches - does it mean >>>>>>> these depends on Dave changes as well? >>>>>> Yes and no. My rework doesn't change anything with this dependency. It >>>>>> comes from my patch "drm: exynos: dsi: Restore proper bridge chain >>>>>> order" already included in your series (patch #1). Without it exynos-dsi >>>>>> driver hacks the list of bridges to ensure the order of pre_enable calls >>>>>> needed for proper operation. This works somehow with DSI panels on my >>>>>> test systems, but it has been reported that it doesn't work with a bit >>>>>> more complex display pipelines. Only that patch depends on the Dave's >>>>>> patches. If you remove it, you would need to adjust the code in the >>>>>> exynos_drm_dsi.c and samsung-dsim.c respectively. imho it would be >>>>>> better to keep it and merge Dave's patches together with dsi changes, as >>>>>> they are the first real client of it. >>>>> I think the Dave patches especially "drm/bridge: Introduce >>>>> pre_enable_upstream_first to alter bridge init order" seems not 100% >>>>> relevant to this series as they affect bridge chain call flow >>>>> globally. Having a separate series for that makes sense to me. I'm >>>>> sending v5 by excluding those parts. >>>> If so then drop the "drm: exynos: dsi: Restore proper bridge chain >>>> order" patch and adjust code respectively in samsung-dsim.c. Without the >>>> Dave's patches, that one doesn't make sense. >>> Doesn't it break Exynos? >> No it won't. Lack of the "drm: exynos: dsi: Restore proper bridge chain >> order" patch doesn't change much against the current state of the driver. >> >> Here is my rework of your v4 patchset without the mentioned patch and >> Dave's patches: >> >> https://protect2.fireeye.com/v1/url?k=6282936d-3d19aa74-62831822-000babff3793-9317af6e2b207460&q=1&e=cd36cc51-8faa-4812-880a-8242739a86bd&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Ftree%2Fv6.0-dsi-v4-reworked-minimal > We have one problem with getting bus format from previous bridge if we > pass NULL in bridge_func.attach() > https://protect2.fireeye.com/v1/url?k=bdc3e18a-e258d893-bdc26ac5-000babff3793-1dd81e5acf9d7f83&q=1&e=cd36cc51-8faa-4812-880a-8242739a86bd&u=https%3A%2F%2Fgithub.com%2Fmszyprow%2Flinux%2Fcommit%2F0fa57e33b3bf866efc4c17ab20eec28d6e07b3e9%23diff-3fe873f1ada5f1dfcf2a50ac114bdab3ea7b026d12278648ca40809d3fa1a331R1321 > > Booting the video as it assigns default bus format if the previous bus > format is unknown. > > [ 1.635984] samsung-dsim 32e10000.dsi: > [drm:samsung_dsim_host_attach] Attached sn65dsi83 device > [ 1.648067] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > 32e00000.lcdif on minor 0 > [ 1.658726] mmc0: SDHCI controller on 30b40000.mmc [30b40000.mmc] > using ADMA > [ 1.681893] sn65dsi83 3-002c: Unsupported LVDS bus format 0x100a, > please check output bridge driver. Falling back to SPWG24. > > Does passing the bridge to drm_bridge_attach is working on your platform? > return drm_bridge_attach(bridge->encoder, dsi->out_bridge, bridge, flags); Nope, it fails: [drm] Initialized exynos 1.1.0 20180330 for exynos-drm on minor 0 exynos-dsi 11c80000.dsi: [drm:samsung_dsim_host_attach] Attached s6e8aa0 device panel-samsung-s6e8aa0 11c80000.dsi.0: error -22 setting maximum return packet size to 3 panel-samsung-s6e8aa0 11c80000.dsi.0: read id failed I've already pointed that it makes sense only together with Dave's patches. Best regards