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

Jagan Teki
2022-10-28T12:13:56Z

On Tue, Oct 18, 2022 at 8:35 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Mon, Oct 17, 2022 at 2:31 PM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
> >
> > 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"?

I'm using this commit head for next version patches, hope all Okay with it.

Thanks,
Jagan.