[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <zr2t6deyvwacawj7s36gols2vxu24fah25x6ofy7xpqyvc4s2d@luavybrlxpaf>
Date: Sat, 27 Jul 2024 23:28:02 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Sui Jingfeng <sui.jingfeng@...ux.dev>
Cc: Liu Ying <victor.liu@....com>, dri-devel@...ts.freedesktop.org,
devicetree@...r.kernel.org, imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org, p.zabel@...gutronix.de,
airlied@...il.com, daniel@...ll.ch, maarten.lankhorst@...ux.intel.com,
mripard@...nel.org, tzimmermann@...e.de, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, shawnguo@...nel.org, s.hauer@...gutronix.de,
kernel@...gutronix.de, festevam@...il.com, tglx@...utronix.de, vkoul@...nel.org,
kishon@...nel.org, aisheng.dong@....com, agx@...xcpu.org, francesco@...cini.it,
frank.li@....com
Subject: Re: [PATCH v3 00/19] Add Freescale i.MX8qxp Display Controller
support
On Sun, Jul 28, 2024 at 03:10:21AM GMT, Sui Jingfeng wrote:
> Hi,
>
> On 7/28/24 00:39, Dmitry Baryshkov wrote:
> > > Hi,
> > >
> > > This patch series aims to add Freescale i.MX8qxp Display Controller support.
> > >
> > > The controller is comprised of three main components that include a blit
> > > engine for 2D graphics accelerations, display controller for display output
> > > processing, as well as a command sequencer.
> > >
> > > Previous patch series attempts to do that can be found at:
> > > https://patchwork.freedesktop.org/series/84524/
> > >
> > > This series addresses Maxime's comments on the previous one:
> > > a. Split the display controller into multiple internal devices.
> > > 1) List display engine, pixel engine, interrupt controller and more as the
> > > controller's child devices.
> > > 2) List display engine and pixel engine's processing units as their child
> > > devices.
> > >
> > > b. Add minimal feature support.
> > > Only support two display pipelines with primary planes with XR24 fb,
> > > backed by two fetchunits. No fetchunit dynamic allocation logic(to be done
> > > when necessary).
> > >
> > > c. Use drm_dev_{enter, exit}().
> > >
> > > Since this series changes a lot comparing to the previous one, I choose to
> > > send it with a new patch series, not a new version.
> > I'm sorry, I have started reviewing v2 without noticing that there is a
> > v3 already.
> >
> > Let me summarize my comments:
> >
> > - You are using OF aliases. Are they documented and acked by DT
> > maintainers?
> >
> > - I generally feel that the use of so many small devices to declare
> > functional blocks is an abuse of the DT. Please consider creating
> > _small_ units from the driver code directly rather than going throught
> > the components.
>
> Well, I really don't think so. I don't agree.
>
> I have checked the DTSpec[1] before type, the spec isn't define how
> many is considered to be "many", and the spec isn't define to what
> extent is think to be "small" as well.
Yeah. However _usually_ we are not defining DT devices for sub-device
components. So at least such decisions ought to be described and
explained in the cover letter.
>
> [1]
> https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.4
--
With best wishes
Dmitry
Powered by blists - more mailing lists