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
