Message ID | 20241002-auto9-v1-0-c4dc3385f415@samsung.com (mailing list archive) |
---|---|
Headers | show |
Series | drm/exynos: Add autov9 DPU code | expand |
Thanks for your contribution. Below are some comments. 2024년 10월 2일 (수) 오후 2:33, Kwanghoon Son <k.son@samsung.com>님이 작성: > > Disclaimer: This code is WIP, not even ready for review. > If you have no interest in Exynos DRM, don't waste your time to read > it. > > 1. why share this ugly code? > > This patch is to share and discuss the necessary parts for recent > upstream of exynos dpu_dma, dpp[1] > Please don't look Patch 2-7, this is just proof that dpu_dma, > dpp works. (Also has build break!) > Also, since I don't have much experience in Linux kernel yet, I thought > it's better to share before going too wrong direction. > This patch is a code that successfully boots, modetest and drm from the > latest v6.12-rc1 base. Good try. If you're uncertain about a patch you've written, don't hesitate to post it as a Work In Progress (WIP) to seek feedback and assistance from the community. This is a valuable approach to contributing to open source projects. > > [1] https://lore.kernel.org/linux-samsung-soc/2e4d3d180f535e57d9cb98e7bac1d14b51ffc5d4.camel@gmail.com/ > > 2. some want to help or discuss > > -Naming problem > Q: What is best naming for files and Kconfig var? > > As-is : exynos9_decon.c exynos_dpp.c exynos_dpu_dma.c > > For DMA, there already exists an exynos_drm_dma.c, so I named it as > 'dpu_dma' according to the IP manual. > But I'm considering make prefix 'exynosautov9_drm' for all or > 'exynos_dpu'. To share an insight regarding the Exynos SoC architecture, until the Exynos5 series, the Exynos SoC family had a display controller (either DECON for Exynos5 series or FIMD for Exynos4 series) that internally included a DMA unit composed of multiple channels, with a separate pre-processor unit (such as FIMC or GSC) existing as a distinct IP. However, in the case of the ExynosAuto series, it appears that the DMA IP (DPU DMA), pre-processing IP (DPP), and display controller IP (DECON) are integrated into a single hardware block and designed to be hardware-wired internally. As a result, it seems that these three IPs must always be enabled simultaneously. Regarding file naming, I would like to offer some insights. Starting from the ExynosAuto series, there has been a significant change in the display block structure from a hardware perspective. While the DECON IP name existed previously, the key difference is that the earlier DECON IP also controlled the DMA controller unit within the driver. This means that the previous DECON driver cannot be universally applied to the new structure. Additionally, historically, the Exynos DRM driver has followed a specific convention when determining file names, using the IP name as a prefix. There are two rules for naming, as outlined below: 1. If two or more Exynos families share the same display block hardware structure, the prefix is based on the lowest version of the Exynos series (e.g., Exynos8 or Exynos8xx). This is because multiple SoCs should be managed by the same driver. In such cases, a compatible string is used to distinguish between the SoCs, allowing the driver to identify the specific IP for each SoC at runtime. 2. If a new display block structure is introduced with a new SoC, the driver name can be determined by using the SoC version as the prefix. However, upon reviewing the device tree code, I noticed that the three IPs share the same base address for memory-mapped I/O like below. decon@18c30000 dpp@18c20000 dpu-dma@18c60000 In my opinion, these three IPs could be integrated into a single module, Exynos8_drm_decon.c or other proper module name. As you can see, same base address, 0x18c00000, are used for three IPs and only difference is offset. > > -DECON interface > Q: dpp_update(), dpu_dma_update() function are looks ok? > > Origin vendor code split each channel to comaptible data(dma_fg0, > dma_g0, ...) using binding. > Problem is Exynos850 or each SOC has own dma,dpp wiring. (e.g. some > Exynos850 is not wiring with dma <-> dpp. they just dma <-> decon) > > So I changed it to code level that decon call register setting > function. > Decon will control dpp, dma based on channel parameter using > dpp_update(), dpu_dma_update() function > > -DP > I'm not familiar with almost all parts of DP. If someone is interested > in Exynos DP, any advice would be appreciated. The DP driver was originally implemented in the drm/exynos folder, but since the Rockchip RK3288 SoC uses the same DP, it was moved to the drm/bridge folder for sharing purposes. Therefore, for Exynos DRM driver dependencies, it would be advisable to add the compatible string for the ExynosAuto SoC version in the exynos5_dp.c module, and implement the SoC-specific code under the drm/bridge/analogx/ folder. For more details, you can refer to below link, https://lwn.net/Articles/656254/ Thanks, Inki Dae > > 3. About exynosv9 arhitecture > > In the case of the existing DRM 5 series, DECON played a role as CRTC > and sent data to the panel. > I don't know about the DRM 7 series, but the Exynos 8 and auto9 series have > DECON divided into three areas: DMA, DPP, and DECON, to started > supporting more channels and formats. > > Exynos auto v9 has > > DPU_DMA - reads the image data from external memory, converts the image > into pixels, and transfers them to remaining display processing > pipeline > DPP (Display Pre-Processor) - frame data from image DMA and applies the > selected image processing function before transferring it to DECON > DECON (Display and Enhancement Controller) - same as previous decon > role. > > and dma and dpp are wired with hardware (They can not mix with other > channel) > > channel> sysMMU> DMA> DPP > ======================= > ch0 MMU_0 GF0 GF0 > ch1 MMU_0 G0 GF1 > ch2 MMU_1 G1 GF2 > ch3 MMU_1 GF1 GF3 > ch4 MMU_2 VG0 VG0 > ch5 MMU_2 G2 GF4 > ch6 MMU_3 G3 GF5 > ch7 MMU_3 VG1 VG1 > > note that exynos850 has 4 channel with different wiring. > > 4. Todos before RFC > > - remove all vendor style code > - more features (resolution, format, plane) > - proper way register setting for dma, dpp channel > - make DP code to proper mainline code using helper functions > > To: Inki Dae <inki.dae@samsung.com> > To: Krzysztof Kozlowski <krzk@kernel.org> > To: Alim Akhtar <alim.akhtar@samsung.com> > To: David Virag <virag.david003@gmail.com> > To: Sam Protsenko <semen.protsenko@linaro.org> > Cc: linux-samsung-soc@vger.kernel.org > > Signed-off-by: Kwanghoon Son <k.son@samsung.com> > --- > Kwanghoon Son (7): > drm/exynos: Initial Exynosautov9 drm > drm/exynos: Add Exynosautov9 decon > drm/exynos: Add drivers on drv.c > drm/exynos: exynos DPTX hw > clk: samsung: exynoautov9: Add dptx cmu > phy: samsung,dp-video-phy: Add exynosautov9 dp phy > arm64: dts: exynosautov9: Enable drm > > arch/arm64/boot/dts/exynos/exynosautov9-dpu.dtsi | 110 + > arch/arm64/boot/dts/exynos/exynosautov9.dtsi | 30 + > drivers/clk/samsung/clk-exynosautov9.c | 35 + > drivers/gpu/drm/exynos/Kconfig | 17 + > drivers/gpu/drm/exynos/Makefile | 3 + > drivers/gpu/drm/exynos/exynos9_decon.c | 1765 +++++++ > drivers/gpu/drm/exynos/exynos_dpp.c | 96 + > drivers/gpu/drm/exynos/exynos_dpp.h | 278 ++ > drivers/gpu/drm/exynos/exynos_dpu_dma.c | 330 ++ > drivers/gpu/drm/exynos/exynos_dpu_dma.h | 16 + > drivers/gpu/drm/exynos/exynos_drm_dp.c | 5038 ++++++++++++++++++++ > drivers/gpu/drm/exynos/exynos_drm_dp.h | 964 ++++ > .../gpu/drm/exynos/exynos_drm_dp_link_training.c | 586 +++ > drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 + > drivers/gpu/drm/exynos/regs-decon9.h | 1244 +++++ > drivers/phy/samsung/phy-exynos-dp-video.c | 9 +- > include/dt-bindings/clock/samsung,exynosautov9.h | 4 + > 18 files changed, 10537 insertions(+), 2 deletions(-) > --- > base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > change-id: 20241002-auto9-961d6a19be50 > > Best regards, > -- > Kwanghoon Son <k.son@samsung.com> > >
Disclaimer: This code is WIP, not even ready for review. If you have no interest in Exynos DRM, don't waste your time to read it. 1. why share this ugly code? This patch is to share and discuss the necessary parts for recent upstream of exynos dpu_dma, dpp[1] Please don't look Patch 2-7, this is just proof that dpu_dma, dpp works. (Also has build break!) Also, since I don't have much experience in Linux kernel yet, I thought it's better to share before going too wrong direction. This patch is a code that successfully boots, modetest and drm from the latest v6.12-rc1 base. [1] https://lore.kernel.org/linux-samsung-soc/2e4d3d180f535e57d9cb98e7bac1d14b51ffc5d4.camel@gmail.com/ 2. some want to help or discuss -Naming problem Q: What is best naming for files and Kconfig var? As-is : exynos9_decon.c exynos_dpp.c exynos_dpu_dma.c For DMA, there already exists an exynos_drm_dma.c, so I named it as 'dpu_dma' according to the IP manual. But I'm considering make prefix 'exynosautov9_drm' for all or 'exynos_dpu'. -DECON interface Q: dpp_update(), dpu_dma_update() function are looks ok? Origin vendor code split each channel to comaptible data(dma_fg0, dma_g0, ...) using binding. Problem is Exynos850 or each SOC has own dma,dpp wiring. (e.g. some Exynos850 is not wiring with dma <-> dpp. they just dma <-> decon) So I changed it to code level that decon call register setting function. Decon will control dpp, dma based on channel parameter using dpp_update(), dpu_dma_update() function -DP I'm not familiar with almost all parts of DP. If someone is interested in Exynos DP, any advice would be appreciated. 3. About exynosv9 arhitecture In the case of the existing DRM 5 series, DECON played a role as CRTC and sent data to the panel. I don't know about the DRM 7 series, but the Exynos 8 and auto9 series have DECON divided into three areas: DMA, DPP, and DECON, to started supporting more channels and formats. Exynos auto v9 has DPU_DMA - reads the image data from external memory, converts the image into pixels, and transfers them to remaining display processing pipeline DPP (Display Pre-Processor) - frame data from image DMA and applies the selected image processing function before transferring it to DECON DECON (Display and Enhancement Controller) - same as previous decon role. and dma and dpp are wired with hardware (They can not mix with other channel) channel> sysMMU> DMA> DPP ======================= ch0 MMU_0 GF0 GF0 ch1 MMU_0 G0 GF1 ch2 MMU_1 G1 GF2 ch3 MMU_1 GF1 GF3 ch4 MMU_2 VG0 VG0 ch5 MMU_2 G2 GF4 ch6 MMU_3 G3 GF5 ch7 MMU_3 VG1 VG1 note that exynos850 has 4 channel with different wiring. 4. Todos before RFC - remove all vendor style code - more features (resolution, format, plane) - proper way register setting for dma, dpp channel - make DP code to proper mainline code using helper functions To: Inki Dae <inki.dae@samsung.com> To: Krzysztof Kozlowski <krzk@kernel.org> To: Alim Akhtar <alim.akhtar@samsung.com> To: David Virag <virag.david003@gmail.com> To: Sam Protsenko <semen.protsenko@linaro.org> Cc: linux-samsung-soc@vger.kernel.org Signed-off-by: Kwanghoon Son <k.son@samsung.com> --- Kwanghoon Son (7): drm/exynos: Initial Exynosautov9 drm drm/exynos: Add Exynosautov9 decon drm/exynos: Add drivers on drv.c drm/exynos: exynos DPTX hw clk: samsung: exynoautov9: Add dptx cmu phy: samsung,dp-video-phy: Add exynosautov9 dp phy arm64: dts: exynosautov9: Enable drm arch/arm64/boot/dts/exynos/exynosautov9-dpu.dtsi | 110 + arch/arm64/boot/dts/exynos/exynosautov9.dtsi | 30 + drivers/clk/samsung/clk-exynosautov9.c | 35 + drivers/gpu/drm/exynos/Kconfig | 17 + drivers/gpu/drm/exynos/Makefile | 3 + drivers/gpu/drm/exynos/exynos9_decon.c | 1765 +++++++ drivers/gpu/drm/exynos/exynos_dpp.c | 96 + drivers/gpu/drm/exynos/exynos_dpp.h | 278 ++ drivers/gpu/drm/exynos/exynos_dpu_dma.c | 330 ++ drivers/gpu/drm/exynos/exynos_dpu_dma.h | 16 + drivers/gpu/drm/exynos/exynos_drm_dp.c | 5038 ++++++++++++++++++++ drivers/gpu/drm/exynos/exynos_drm_dp.h | 964 ++++ .../gpu/drm/exynos/exynos_drm_dp_link_training.c | 586 +++ drivers/gpu/drm/exynos/exynos_drm_drv.c | 11 +- drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 + drivers/gpu/drm/exynos/regs-decon9.h | 1244 +++++ drivers/phy/samsung/phy-exynos-dp-video.c | 9 +- include/dt-bindings/clock/samsung,exynosautov9.h | 4 + 18 files changed, 10537 insertions(+), 2 deletions(-) --- base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc change-id: 20241002-auto9-961d6a19be50 Best regards,