| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e407affd-84a8-4556-8707-65ec79455317@nxp.com>
Date: Tue, 30 Jul 2024 16:42:09 +0800
From: Liu Ying <victor.liu@....com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: 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 07/28/2024, Dmitry Baryshkov wrote:
> On Wed, Jul 24, 2024 at 05:29:31PM GMT, Liu Ying 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.
I've replied your comments on v2.
Thanks for your review!
>
> Let me summarize my comments:
>
> - You are using OF aliases. Are they documented and acked by DT
> maintainers?
As I replied in v2, I may document them if needed.
No Nack/Ack from DT maintainers as of now.
>
> - 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. Also please describe how everything fits together in
> the cover letter.
As I replied in v2, they are modelled as separated devices and kinda
accepted by Maxime and Rob.
I'll try to describe more in cover letter.
>
> - I assume that there more functional units that you are cunrretly
> adding and there is more versatility. Please describe that in the
> commit messages.
I've documented all processing units and the other devices in the
main display controller, nothing more.
I'll list all processing units in commit message in next version.
>
> - I see a lot of small functions, which can be inlined without the lost
> of code clarify. Please consider self-reviewing your code from this
> perspective.
Can you please point out typical ones in patches in question?
>
> - There were other small comments, but I think they are less important
> now. You might still consider them for v4.
Thanks again for your review.
>
>> To follow up i.MX8qxp TRM, I changed the controller name to "Display Controller"
>> instead of the previous "DPU". "DPU" is only mentioned in the SoC block
>> diagram and represents the whole display subsystem which includes the display
>> controller and prefech engines, etc.
>>
>> With an additional patch[1] for simple-pm-bus.c, this series facilitates
>> testing a LVDS panel on i.MX8qxp MEK.
>>
>> Please do NOT merge patch 14-19.
>>
>> [1] https://lkml.org/lkml/2023/1/25/120
>>
>> v3:
>> * Collect Rob's R-b tag on the patch for adding fsl,imx8qxp-dc-intc.yaml.
>> * Combine fsl,imx8qxp-dc-fetchunit-common.yaml,
>> fsl,imx8qxp-dc-fetchlayer.yaml and fsl,imx8qxp-dc-fetchwarp.yaml
>> into 1 schema doc fsl,imx8qxp-dc-fetchunit.yaml. (Rob)
>> * Document all processing units, command sequencer, axi performance counter
>> and blit engine. (Rob)
>>
>> v2:
>> * Drop fsl,dc-*-id DT properties from fsl,imx8qxp-dc*.yaml. (Krzysztof)
>> * Move port property from fsl,imx8qxp-dc-display-engine.yaml to
>> fsl,imx8qxp-dc-tcon.yaml. (Krzysztof)
>> * Drop unneeded "|" from fsl,imx8qxp-dc-intc.yaml. (Krzysztof)
>> * Use generic pmu pattern property in fsl,imx8qxp-dc.yaml. (Krzysztof)
>> * Fix register range size in fsl,imx8qxp-dc*.yaml.
>> * Use OF alias id to get instance id from display driver.
>> * Find next bridge from TCon's port from display driver.
>> * Drop drm/drm_module.h include from dc-drv.c.
>> * Improve file list in MAINTAINERS. (Frank)
>> * Add entire i.MX8qxp display controller device tree for review. (Krzysztof)
>> * Add MIPI/LVDS subsystems device tree and a DT overlay for imx8qxp
>> MEK to test a LVDS panel as an example. (Francesco)
>>
>> Liu Ying (19):
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller processing
>> units
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller blit engine
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller display
>> engine
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller pixel
>> engine
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller AXI
>> performance counter
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller command
>> sequencer
>> dt-bindings: interrupt-controller: Add i.MX8qxp Display Controller
>> interrupt controller
>> dt-bindings: display: imx: Add i.MX8qxp Display Controller
>> drm/imx: Add i.MX8qxp Display Controller display engine
>> drm/imx: Add i.MX8qxp Display Controller pixel engine
>> drm/imx: Add i.MX8qxp Display Controller interrupt controller
>> drm/imx: Add i.MX8qxp Display Controller KMS
>> MAINTAINERS: Add maintainer for i.MX8qxp Display Controller
>> dt-bindings: phy: mixel,mipi-dsi-phy: Allow assigned-clock* properties
>> dt-bindings: firmware: imx: Add SCU controlled display pixel link
>> nodes
>> arm64: dts: imx8qxp: Add display controller subsystem
>> arm64: dts: imx8qxp: Add MIPI-LVDS combo subsystems
>> arm64: dts: imx8qxp-mek: Enable display controller
>> arm64: dts: imx8qxp-mek: Add MX8-DLVDS-LCD1 display module support
>>
>> ...sl,imx8qxp-dc-axi-performance-counter.yaml | 57 ++
>> .../imx/fsl,imx8qxp-dc-blit-engine.yaml | 204 +++++++
>> .../display/imx/fsl,imx8qxp-dc-blitblend.yaml | 41 ++
>> .../display/imx/fsl,imx8qxp-dc-clut.yaml | 44 ++
>> .../imx/fsl,imx8qxp-dc-command-sequencer.yaml | 67 ++
>> .../imx/fsl,imx8qxp-dc-constframe.yaml | 44 ++
>> .../imx/fsl,imx8qxp-dc-display-engine.yaml | 152 +++++
>> .../display/imx/fsl,imx8qxp-dc-dither.yaml | 45 ++
>> .../display/imx/fsl,imx8qxp-dc-extdst.yaml | 72 +++
>> .../display/imx/fsl,imx8qxp-dc-fetchunit.yaml | 141 +++++
>> .../display/imx/fsl,imx8qxp-dc-filter.yaml | 43 ++
>> .../display/imx/fsl,imx8qxp-dc-framegen.yaml | 64 ++
>> .../display/imx/fsl,imx8qxp-dc-gammacor.yaml | 32 +
>> .../imx/fsl,imx8qxp-dc-layerblend.yaml | 39 ++
>> .../display/imx/fsl,imx8qxp-dc-matrix.yaml | 44 ++
>> .../imx/fsl,imx8qxp-dc-pixel-engine.yaml | 250 ++++++++
>> .../display/imx/fsl,imx8qxp-dc-rop.yaml | 43 ++
>> .../display/imx/fsl,imx8qxp-dc-safety.yaml | 34 ++
>> .../imx/fsl,imx8qxp-dc-scaling-engine.yaml | 83 +++
>> .../display/imx/fsl,imx8qxp-dc-signature.yaml | 53 ++
>> .../display/imx/fsl,imx8qxp-dc-store.yaml | 96 +++
>> .../display/imx/fsl,imx8qxp-dc-tcon.yaml | 45 ++
>> .../bindings/display/imx/fsl,imx8qxp-dc.yaml | 236 +++++++
>> .../devicetree/bindings/firmware/fsl,scu.yaml | 20 +
>> .../fsl,imx8qxp-dc-intc.yaml | 318 ++++++++++
>> .../bindings/phy/mixel,mipi-dsi-phy.yaml | 5 -
>> MAINTAINERS | 8 +
>> arch/arm64/boot/dts/freescale/Makefile | 4 +
>> .../arm64/boot/dts/freescale/imx8-ss-dc0.dtsi | 408 +++++++++++++
>> .../imx8qxp-mek-mx8-dlvds-lcd1-lvds0-odd.dtso | 183 ++++++
>> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 34 ++
>> .../boot/dts/freescale/imx8qxp-ss-dc.dtsi | 240 ++++++++
>> .../dts/freescale/imx8qxp-ss-mipi-lvds.dtsi | 437 +++++++++++++
>> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 28 +-
>> drivers/gpu/drm/imx/Kconfig | 1 +
>> drivers/gpu/drm/imx/Makefile | 1 +
>> drivers/gpu/drm/imx/dc/Kconfig | 8 +
>> drivers/gpu/drm/imx/dc/Makefile | 7 +
>> drivers/gpu/drm/imx/dc/dc-cf.c | 157 +++++
>> drivers/gpu/drm/imx/dc/dc-crtc.c | 578 ++++++++++++++++++
>> drivers/gpu/drm/imx/dc/dc-crtc.h | 67 ++
>> drivers/gpu/drm/imx/dc/dc-de.c | 151 +++++
>> drivers/gpu/drm/imx/dc/dc-de.h | 65 ++
>> drivers/gpu/drm/imx/dc/dc-drv.c | 275 +++++++++
>> drivers/gpu/drm/imx/dc/dc-drv.h | 54 ++
>> drivers/gpu/drm/imx/dc/dc-ed.c | 266 ++++++++
>> drivers/gpu/drm/imx/dc/dc-fg.c | 366 +++++++++++
>> drivers/gpu/drm/imx/dc/dc-fl.c | 136 +++++
>> drivers/gpu/drm/imx/dc/dc-fu.c | 241 ++++++++
>> drivers/gpu/drm/imx/dc/dc-fu.h | 129 ++++
>> drivers/gpu/drm/imx/dc/dc-fw.c | 149 +++++
>> drivers/gpu/drm/imx/dc/dc-ic.c | 249 ++++++++
>> drivers/gpu/drm/imx/dc/dc-kms.c | 143 +++++
>> drivers/gpu/drm/imx/dc/dc-kms.h | 15 +
>> drivers/gpu/drm/imx/dc/dc-lb.c | 300 +++++++++
>> drivers/gpu/drm/imx/dc/dc-pe.c | 140 +++++
>> drivers/gpu/drm/imx/dc/dc-pe.h | 91 +++
>> drivers/gpu/drm/imx/dc/dc-plane.c | 227 +++++++
>> drivers/gpu/drm/imx/dc/dc-plane.h | 37 ++
>> drivers/gpu/drm/imx/dc/dc-tc.c | 137 +++++
>> 60 files changed, 7598 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-axi-performance-counter.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-blit-engine.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-blitblend.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-clut.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-command-sequencer.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-constframe.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-display-engine.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-dither.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-extdst.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-fetchunit.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-filter.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-framegen.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-gammacor.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-layerblend.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-matrix.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-pixel-engine.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-rop.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-safety.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-scaling-engine.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-signature.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-store.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc-tcon.yaml
>> create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8qxp-dc.yaml
>> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/fsl,imx8qxp-dc-intc.yaml
>> create mode 100644 arch/arm64/boot/dts/freescale/imx8-ss-dc0.dtsi
>> create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-mek-mx8-dlvds-lcd1-lvds0-odd.dtso
>> create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-dc.dtsi
>> create mode 100644 arch/arm64/boot/dts/freescale/imx8qxp-ss-mipi-lvds.dtsi
>> create mode 100644 drivers/gpu/drm/imx/dc/Kconfig
>> create mode 100644 drivers/gpu/drm/imx/dc/Makefile
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-cf.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-crtc.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-crtc.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-de.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-de.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-drv.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-drv.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-ed.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-fg.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-fl.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-fu.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-fu.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-fw.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-ic.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-kms.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-kms.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-lb.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-pe.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-pe.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-plane.c
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-plane.h
>> create mode 100644 drivers/gpu/drm/imx/dc/dc-tc.c
>>
>> --
>> 2.34.1
>>
>
--
Regards,
Liu Ying
Powered by blists - more mailing lists