On Fri, Sep 16, 2022 at 1:58 PM Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> 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@xxxxxxxxxxx> wrote: > >> On 13.09.2022 19:29, Jagan Teki wrote: > >>> On Wed, Sep 7, 2022 at 3:34 PM Marek Szyprowski > >>> <m.szyprowski@xxxxxxxxxxx> wrote: > >>>> On 06.09.2022 21:07, Jagan Teki wrote: > >>>>> On Mon, Sep 5, 2022 at 4:54 PM Marek Szyprowski > >>>>> <m.szyprowski@xxxxxxxxxxx> 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.
