Re: [PATCH v7 01/10] drm: bridge: Add Samsung DSIM bridge driver

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Marek Vasut <marex@denx.de>, Jagan Teki <jagan@amarulasolutions.com>
Cc: dri-devel@lists.freedesktop.org,
	linux-samsung-soc@vger.kernel.org,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Tommaso Merciai <tommaso.merciai@amarulasolutions.com>,
	linux-amarula <linux-amarula@amarulasolutions.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Neil Armstrong <narmstrong@linaro.org>,
	Frieder Schrempf <frieder.schrempf@kontron.de>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Matteo Lisi <matteo.lisi@engicam.com>,
	Robert Foss <robert.foss@linaro.org>,
	Andrzej Hajda <andrzej.hajda@intel.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Fancy Fang <chen.fang@nxp.com>,
	Michael Nazzareno Trimarchi <michael@amarulasolutions.com>,
	Adam Ford <aford173@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH v7 01/10] drm: bridge: Add Samsung DSIM bridge driver
Date: Mon, 17 Oct 2022 11:01:10 +0200	[thread overview]
Message-ID: <2b0350ba-070a-8df0-9a2d-8c18da03b6cb@samsung.com> (raw)
In-Reply-To: <76423028-e58e-7271-0d74-fb4bb2bf65c8@denx.de>

Hi,

On 17.10.2022 10:48, Marek Vasut wrote:
> On 10/17/22 09:43, Jagan Teki wrote:
>> On Mon, Oct 17, 2022 at 12:49 PM Marek Vasut <marex@denx.de> wrote:
>>> On 10/17/22 04:49, Jagan Teki wrote:
>>>> On Sun, Oct 16, 2022 at 3:16 AM Marek Vasut <marex@denx.de> wrote:
>>>>>
>>>>> On 10/5/22 17:13, 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 by maintaining exynos platform glue code in exynos_drm_dsi.c 
>>>>>> driver
>>>>>> and common bridge driver code in samsung-dsim.c providing that 
>>>>>> the new
>>>>>> platform-specific glue should be supported in the bridge driver, 
>>>>>> unlike
>>>>>> exynos platform drm drivers.
>>>>>>
>>>>>> - Add samsung_dsim_plat_data for keeping platform-specific 
>>>>>> attributes like
>>>>>>      host_ops, irq_ops, and hw_type.
>>>>>>
>>>>>> - Initialize the plat_data hooks for exynos platform in 
>>>>>> exynos_drm_dsi.c.
>>>>>>
>>>>>> - samsung_dsim_probe is the common probe call across 
>>>>>> exynos_drm_dsi.c and
>>>>>>      samsung-dsim.c.
>>>>>>
>>>>>> - plat_data hooks like host_ops and irq_ops are invoked during the
>>>>>>      respective bridge call chains.
>>>>>
>>>>> Maybe the Subject should say "Split ... driver" or "Move ... 
>>>>> driver" ,
>>>>> since it is not adding a new driver here ?
>>>>
>>>> Though it is not added a completely new driver, it is adding more
>>>> infrastructure platform code to be compatible with both Exynos and
>>>> i.MX8M. This is the prime reason for adding that commit head and
>>>> explaining the same in the commit body.
>>>
>>> Diffstat looks like this:
>>>
>>>    drivers/gpu/drm/bridge/samsung-dsim.c   | 1703 
>>> ++++++++++++++++++++++
>>>    drivers/gpu/drm/exynos/Kconfig          |    1 +
>>>    drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1766 
>>> ++---------------------
>>>    include/drm/bridge/samsung-dsim.h       |  113 ++
>>>    7 files changed, 1952 insertions(+), 1653 deletions(-)
>>>
>>> Looks to me like most of the code is just moved from existing driver in
>>> this patch.
>>
>> Yeah, as I explained (from commit) it is moved, updated, and written
>> the plat code. How about this head?
>>
>> "drm: bridge: Add Samsung DSIM bridge (Split from exynos_drm_dsi)"
>
> I disagree with the "Add" part of the Subject, but I'll wait for 
> others' opinion here.

Maybe something like a "Generalize Exynos-DSI DRM driver into a generic 
Samsung DSIM bridge"?

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland