[PATCH v4 02/12] drm: bridge: Add Samsung DSIM bridge driver

[Frieder Schrempf]

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