| Message ID | 20250110-drm-s4-v1-0-cbc2d5edaae8@amlogic.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Subject: [PATCH 00/11] Add DRM support for Amlogic S4 | expand |
Hi, On 10/01/2025 06:39, Ao Xu via B4 Relay wrote: > This patch series adds DRM support for the Amlogic S4-series SoCs. > Compared to the Amlogic G12-series, the S4-series introduces the following changes: > > 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. > As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. > 2 The S4-series secures access to HDMITX DWC and TOP registers, > requiring modifications to the driver to handle this new access method. > 3 The register addresses for the video1 and video2 planes have been updated in the S4 hardware, > and the DRM driver has been adapted accordingly. > 4 The OSD, VIU, and VPP components remain unchanged and are consistent with the G12-series. Thanks a lot for this high quality changeset, happy to see DRM support for a new SoC ! I'll review it carefully next week. Neil > > Signed-off-by: Ao Xu <ao.xu@amlogic.com> > --- > Ao Xu (11): > dt-bindings: display: meson-dw-hdmi: Add compatible for S4 HDMI controller > dt-bindings: display: meson-vpu: Add compatible for S4 display controller > drm: meson: add S4 compatible for DRM driver > drm: meson: add primary and overlay plane support for S4 > drm: meson: update VIU and VPP support for S4 > drm: meson: add meson_dw_hdmi support for S4 > drm: meson: change api call parameter > drm: meson: add hdmitx vmode timing support for S4 > drm: meson: add vpu clk setting for S4 > drm: meson: add CVBS support for S4 > arm64: dts: amlogic: s4: add DRM support [1/1] > > .../bindings/display/amlogic,meson-dw-hdmi.yaml | 1 + > .../bindings/display/amlogic,meson-vpu.yaml | 48 +- > .../boot/dts/amlogic/meson-s4-s805x2-aq222.dts | 39 + > arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 121 +++ > drivers/gpu/drm/meson/meson_crtc.c | 90 +- > drivers/gpu/drm/meson/meson_drv.c | 127 ++- > drivers/gpu/drm/meson/meson_drv.h | 6 + > drivers/gpu/drm/meson/meson_dw_hdmi.c | 244 ++++- > drivers/gpu/drm/meson/meson_dw_hdmi.h | 126 +++ > drivers/gpu/drm/meson/meson_encoder_cvbs.c | 10 + > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 19 +- > drivers/gpu/drm/meson/meson_overlay.c | 7 +- > drivers/gpu/drm/meson/meson_plane.c | 24 +- > drivers/gpu/drm/meson/meson_registers.h | 17 + > drivers/gpu/drm/meson/meson_vclk.c | 1018 ++++++++++++++------ > drivers/gpu/drm/meson/meson_venc.c | 346 ++++++- > drivers/gpu/drm/meson/meson_venc.h | 4 +- > drivers/gpu/drm/meson/meson_viu.c | 9 +- > drivers/gpu/drm/meson/meson_vpp.c | 12 +- > 19 files changed, 1865 insertions(+), 403 deletions(-) > --- > base-commit: 6ecd20965bdc21b265a0671ccf36d9ad8043f5ab > change-id: 20250110-drm-s4-c96c88be52e4 > > Best regards,
On Fri, 10 Jan 2025 13:39:50 +0800, Ao Xu wrote: > This patch series adds DRM support for the Amlogic S4-series SoCs. > Compared to the Amlogic G12-series, the S4-series introduces the following changes: > > 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. > As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. > 2 The S4-series secures access to HDMITX DWC and TOP registers, > requiring modifications to the driver to handle this new access method. > 3 The register addresses for the video1 and video2 planes have been updated in the S4 hardware, > and the DRM driver has been adapted accordingly. > 4 The OSD, VIU, and VPP components remain unchanged and are consistent with the G12-series. > > Signed-off-by: Ao Xu <ao.xu@amlogic.com> > --- > Ao Xu (11): > dt-bindings: display: meson-dw-hdmi: Add compatible for S4 HDMI controller > dt-bindings: display: meson-vpu: Add compatible for S4 display controller > drm: meson: add S4 compatible for DRM driver > drm: meson: add primary and overlay plane support for S4 > drm: meson: update VIU and VPP support for S4 > drm: meson: add meson_dw_hdmi support for S4 > drm: meson: change api call parameter > drm: meson: add hdmitx vmode timing support for S4 > drm: meson: add vpu clk setting for S4 > drm: meson: add CVBS support for S4 > arm64: dts: amlogic: s4: add DRM support [1/1] > > .../bindings/display/amlogic,meson-dw-hdmi.yaml | 1 + > .../bindings/display/amlogic,meson-vpu.yaml | 48 +- > .../boot/dts/amlogic/meson-s4-s805x2-aq222.dts | 39 + > arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 121 +++ > drivers/gpu/drm/meson/meson_crtc.c | 90 +- > drivers/gpu/drm/meson/meson_drv.c | 127 ++- > drivers/gpu/drm/meson/meson_drv.h | 6 + > drivers/gpu/drm/meson/meson_dw_hdmi.c | 244 ++++- > drivers/gpu/drm/meson/meson_dw_hdmi.h | 126 +++ > drivers/gpu/drm/meson/meson_encoder_cvbs.c | 10 + > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 19 +- > drivers/gpu/drm/meson/meson_overlay.c | 7 +- > drivers/gpu/drm/meson/meson_plane.c | 24 +- > drivers/gpu/drm/meson/meson_registers.h | 17 + > drivers/gpu/drm/meson/meson_vclk.c | 1018 ++++++++++++++------ > drivers/gpu/drm/meson/meson_venc.c | 346 ++++++- > drivers/gpu/drm/meson/meson_venc.h | 4 +- > drivers/gpu/drm/meson/meson_viu.c | 9 +- > drivers/gpu/drm/meson/meson_vpp.c | 12 +- > 19 files changed, 1865 insertions(+), 403 deletions(-) > --- > base-commit: 6ecd20965bdc21b265a0671ccf36d9ad8043f5ab > change-id: 20250110-drm-s4-c96c88be52e4 > > Best regards, > -- > Ao Xu <ao.xu@amlogic.com> > > > My bot found new DTB warnings on the .dts files added or changed in this series. Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings are fixed by another series. Ultimately, it is up to the platform maintainer whether these warnings are acceptable or not. No need to reply unless the platform maintainer has comments. If you already ran DT checks and didn't see these error(s), then make sure dt-schema is up to date: pip3 install dtschema --upgrade New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/amlogic/' for 20250110-drm-s4-v1-0-cbc2d5edaae8@amlogic.com: arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dtb: vpu@ff000000: reg: [[0, 4278190080, 0, 262144], [0, 4261445632, 0, 8192], [0, 4261412864, 0, 8192], [0, 4261462016, 0, 2048], [0, 4261478400, 0, 256]] is too long from schema $id: http://devicetree.org/schemas/display/amlogic,meson-vpu.yaml# arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dtb: vpu@ff000000: reg-names: ['vpu', 'hhi', 'clkctrl', 'pwrctrl', 'sysctrl'] is too long from schema $id: http://devicetree.org/schemas/display/amlogic,meson-vpu.yaml# arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dtb: vpu@ff000000: reg-names:3: 'pwctrl' was expected from schema $id: http://devicetree.org/schemas/display/amlogic,meson-vpu.yaml# arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dtb: vpu@ff000000: 'assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/display/amlogic,meson-vpu.yaml#
Hello, On Fri, Jan 10, 2025 at 6:39 AM Ao Xu via B4 Relay <devnull+ao.xu.amlogic.com@kernel.org> wrote: > > This patch series adds DRM support for the Amlogic S4-series SoCs. > Compared to the Amlogic G12-series, the S4-series introduces the following changes: Thanks for your patches. With this mail I'll try to summarize my understanding of the situation with the drm/meson driver as I'm not sure how familiar you are with previous discussions. > 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. > As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. Jerome has already commented on patch 3/11 that this mixes in too many IP blocks into one driver. This is not a new situation, the existing code is doing exactly the same. Jerome has previously sent a series which started "an effort to remove the use HHI syscon" [0] from the drm/meson hdmi driver. In the same mail he further notes: "[the patchset] stops short of making any controversial DT changes. To decouple the HDMI phy driver and main DRM driver, allowing the PHY to get hold of its registers, I believe a DT ABI break is inevitable. Ideally the register region of the PHY within the HHI should provided, not the whole HHI. That's pain for another day ..." For now I'm assuming you're familiar with device-tree ABI. If not then please let us know so we can elaborate further on this. My own notes for meson_dw_hdmi.c are: - we should not program HHI_HDMI_CLK_CNTL directly but go through CCF (common clock framework) instead (we should already have the driver for this in place) - we should not program HHI_MEM_PD_REG0 directly but go through the genpd/pmdomain framework (we should already have the driver for this in place) - for the HDMI PHY registers: on Meson8/8b/8m2 (those were 32-bit SoCs in case you're not familiar with them, they predate GXBB/GXL/...) I wrote a PHY driver (which is already upstream: drivers/phy/amlogic/phy-meson8-hdmi-tx.c) as that made most sense to me Then there's meson_venc.c which has two writes to HHI_GCLK_MPEG2. I think those should go through CCF instead of directly accessing this register. Also there's the VDAC registers in meson_encoder_cvbs.c: I think Neil suggested at one point to make it a PHY driver. I even started working on this (if you're curious: see [0] and [1]). DT ABI backwards compatibility is also a concern here. And finally there's the video clock tree programming in meson_vclk.c. My understanding here is that video PLL settings are very sensitive and require fine-tuning according to the desired output clock. Since it's a bunch of clocks I'd say that direct programming of the clock registers should be avoided and we need to go through CCF as well. But to me those register values are all magic (as I am not aware of any documentation that's available to me which describes how to determine the correct PLL register values - otherside the standard en/m/n/frac/lock and reset bits). I'm not saying that all of my thoughts are correct and immediately need to be put into code. Think of them more as an explanation to Jerome's reaction. I think what we need next is a discussion on how to move forward with device-tree ABI for new SoCs. Neil, Jerome: I'd like to hear your feedback on this. > 2 The S4-series secures access to HDMITX DWC and TOP registers, > requiring modifications to the driver to handle this new access method. This info should go into patch 1/11 to explain why the g12a compatible string cannot be used as fallback. Best regards, Martin [0] https://github.com/xdarklight/linux/commit/36e698edc25dc698a0d57b729a7a4587fafc0987 [1] https://github.com/xdarklight/linux/commit/824b792fdbd2d3c0b71b21e1105eca0aaad8daa6
On Sun 12 Jan 2025 at 23:44, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Hello, > > On Fri, Jan 10, 2025 at 6:39 AM Ao Xu via B4 Relay > <devnull+ao.xu.amlogic.com@kernel.org> wrote: >> >> This patch series adds DRM support for the Amlogic S4-series SoCs. >> Compared to the Amlogic G12-series, the S4-series introduces the following changes: > Thanks for your patches. With this mail I'll try to summarize my > understanding of the situation with the drm/meson driver as I'm not > sure how familiar you are with previous discussions. > >> 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. >> As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. > Jerome has already commented on patch 3/11 that this mixes in too many > IP blocks into one driver. > This is not a new situation, the existing code is doing exactly the same. > > Jerome has previously sent a series which started "an effort to remove > the use HHI syscon" [0] from the drm/meson hdmi driver. > In the same mail he further notes: "[the patchset] stops short of > making any controversial DT changes. To decouple the HDMI > phy driver and main DRM driver, allowing the PHY to get hold of its > registers, I believe a DT ABI break is inevitable. Ideally the > register region of the PHY within the HHI should provided, not the > whole HHI. That's pain for another day ..." > > For now I'm assuming you're familiar with device-tree ABI. > If not then please let us know so we can elaborate further on this. > > My own notes for meson_dw_hdmi.c are: > - we should not program HHI_HDMI_CLK_CNTL directly but go through CCF > (common clock framework) instead (we should already have the driver > for this in place) > - we should not program HHI_MEM_PD_REG0 directly but go through the > genpd/pmdomain framework (we should already have the driver for this > in place) > - for the HDMI PHY registers: on Meson8/8b/8m2 (those were 32-bit SoCs > in case you're not familiar with them, they predate GXBB/GXL/...) I > wrote a PHY driver (which is already upstream: > drivers/phy/amlogic/phy-meson8-hdmi-tx.c) as that made most sense to > me > > Then there's meson_venc.c which has two writes to HHI_GCLK_MPEG2. > I think those should go through CCF instead of directly accessing this register. > > Also there's the VDAC registers in meson_encoder_cvbs.c: > I think Neil suggested at one point to make it a PHY driver. I even > started working on this (if you're curious: see [0] and [1]). > DT ABI backwards compatibility is also a concern here. > > And finally there's the video clock tree programming in meson_vclk.c. > My understanding here is that video PLL settings are very sensitive > and require fine-tuning according to the desired output clock. > Since it's a bunch of clocks I'd say that direct programming of the > clock registers should be avoided and we need to go through CCF as > well. > But to me those register values are all magic (as I am not aware of > any documentation that's available to me which describes how to > determine the correct PLL register values - otherside the standard > en/m/n/frac/lock and reset bits). > > I'm not saying that all of my thoughts are correct and immediately > need to be put into code. > Think of them more as an explanation to Jerome's reaction. > > I think what we need next is a discussion on how to move forward with > device-tree ABI for new SoCs. > Neil, Jerome: I'd like to hear your feedback on this. I completely agree with your description of the problem, that and Krzysztof's remark on patch 6. This is not new with this series indeed, so it does not introduce new problems really but it compounds the existing ones. Addressing those issues, if we want to, will get more difficult as more support is added for sure. > >> 2 The S4-series secures access to HDMITX DWC and TOP registers, >> requiring modifications to the driver to handle this new access method. Regmap buses are made for those cases where the registers are the same, but accessed differently. There is an example in the patchset referenced by Martin, to handle GXL and G12 diff. > This info should go into patch 1/11 to explain why the g12a compatible > string cannot be used as fallback. > > > Best regards, > Martin > > > [0] https://github.com/xdarklight/linux/commit/36e698edc25dc698a0d57b729a7a4587fafc0987 > [1] https://github.com/xdarklight/linux/commit/824b792fdbd2d3c0b71b21e1105eca0aaad8daa6
在 2025/1/15 1:50, Jerome Brunet 写道: > [ EXTERNAL EMAIL ] > > On Sun 12 Jan 2025 at 23:44, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > >> Hello, >> >> On Fri, Jan 10, 2025 at 6:39 AM Ao Xu via B4 Relay >> <devnull+ao.xu.amlogic.com@kernel.org> wrote: >>> This patch series adds DRM support for the Amlogic S4-series SoCs. >>> Compared to the Amlogic G12-series, the S4-series introduces the following changes: >> Thanks for your patches. With this mail I'll try to summarize my >> understanding of the situation with the drm/meson driver as I'm not >> sure how familiar you are with previous discussions. >> >>> 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. >>> As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. >> Jerome has already commented on patch 3/11 that this mixes in too many >> IP blocks into one driver. >> This is not a new situation, the existing code is doing exactly the same. >> >> Jerome has previously sent a series which started "an effort to remove >> the use HHI syscon" [0] from the drm/meson hdmi driver. >> In the same mail he further notes: "[the patchset] stops short of >> making any controversial DT changes. To decouple the HDMI >> phy driver and main DRM driver, allowing the PHY to get hold of its >> registers, I believe a DT ABI break is inevitable. Ideally the >> register region of the PHY within the HHI should provided, not the >> whole HHI. That's pain for another day ..." >> >> For now I'm assuming you're familiar with device-tree ABI. >> If not then please let us know so we can elaborate further on this. >> >> My own notes for meson_dw_hdmi.c are: >> - we should not program HHI_HDMI_CLK_CNTL directly but go through CCF >> (common clock framework) instead (we should already have the driver >> for this in place) >> - we should not program HHI_MEM_PD_REG0 directly but go through the >> genpd/pmdomain framework (we should already have the driver for this >> in place) >> - for the HDMI PHY registers: on Meson8/8b/8m2 (those were 32-bit SoCs >> in case you're not familiar with them, they predate GXBB/GXL/...) I >> wrote a PHY driver (which is already upstream: >> drivers/phy/amlogic/phy-meson8-hdmi-tx.c) as that made most sense to >> me >> >> Then there's meson_venc.c which has two writes to HHI_GCLK_MPEG2. >> I think those should go through CCF instead of directly accessing this register. >> >> Also there's the VDAC registers in meson_encoder_cvbs.c: >> I think Neil suggested at one point to make it a PHY driver. I even >> started working on this (if you're curious: see [0] and [1]). >> DT ABI backwards compatibility is also a concern here. >> >> And finally there's the video clock tree programming in meson_vclk.c. >> My understanding here is that video PLL settings are very sensitive >> and require fine-tuning according to the desired output clock. >> Since it's a bunch of clocks I'd say that direct programming of the >> clock registers should be avoided and we need to go through CCF as >> well. >> But to me those register values are all magic (as I am not aware of >> any documentation that's available to me which describes how to >> determine the correct PLL register values - otherside the standard >> en/m/n/frac/lock and reset bits). >> >> I'm not saying that all of my thoughts are correct and immediately >> need to be put into code. >> Think of them more as an explanation to Jerome's reaction. >> >> I think what we need next is a discussion on how to move forward with >> device-tree ABI for new SoCs. >> Neil, Jerome: I'd like to hear your feedback on this. > I completely agree with your description of the problem, that and > Krzysztof's remark on patch 6. This is not new with this series indeed, > so it does not introduce new problems really but it compounds the > existing ones. > > Addressing those issues, if we want to, will get more difficult as more > support is added for sure. Hi, Jerome, Neil, Martin Thanks for your review. I hadn't noticed Jerome's patchsets before, and he has already done some excellent work. Indeed, the current code is overly coupled, with HHI and VPU regmap mixed together. It also contains a lot of redundant code, which is not very conducive to extension and maintenance. We are also considering ways to improve the DRM Meson code, and this is a good attempt. The code should reuse frameworks like CCF, reset, PHY, and PD as much as possible. I'm more than happy to collaborate on restructuring theAmlogic DRM driver to make it better! >>> 2 The S4-series secures access to HDMITX DWC and TOP registers, >>> requiring modifications to the driver to handle this new access method. > Regmap buses are made for those cases where the registers are the same, > but accessed differently. There is an example in the patchset referenced by > Martin, to handle GXL and G12 diff. > >> This info should go into patch 1/11 to explain why the g12a compatible >> string cannot be used as fallback. >> >> >> Best regards, >> Martin >> >> >> [0] https://github.com/xdarklight/linux/commit/36e698edc25dc698a0d57b729a7a4587fafc0987 >> [1] https://github.com/xdarklight/linux/commit/824b792fdbd2d3c0b71b21e1105eca0aaad8daa6 > -- > Jerome
On 2025/1/15 1:50, Jerome Brunet wrote: > [ EXTERNAL EMAIL ] > > On Sun 12 Jan 2025 at 23:44, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > >> Hello, >> >> On Fri, Jan 10, 2025 at 6:39 AM Ao Xu via B4 Relay >> <devnull+ao.xu.amlogic.com@kernel.org> wrote: >>> This patch series adds DRM support for the Amlogic S4-series SoCs. >>> Compared to the Amlogic G12-series, the S4-series introduces the following changes: >> Thanks for your patches. With this mail I'll try to summarize my >> understanding of the situation with the drm/meson driver as I'm not >> sure how familiar you are with previous discussions. >> >>> 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. >>> As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. >> Jerome has already commented on patch 3/11 that this mixes in too many >> IP blocks into one driver. >> This is not a new situation, the existing code is doing exactly the same. >> >> Jerome has previously sent a series which started "an effort to remove >> the use HHI syscon" [0] from the drm/meson hdmi driver. >> In the same mail he further notes: "[the patchset] stops short of >> making any controversial DT changes. To decouple the HDMI >> phy driver and main DRM driver, allowing the PHY to get hold of its >> registers, I believe a DT ABI break is inevitable. Ideally the >> register region of the PHY within the HHI should provided, not the >> whole HHI. That's pain for another day ..." >> >> For now I'm assuming you're familiar with device-tree ABI. >> If not then please let us know so we can elaborate further on this. >> >> My own notes for meson_dw_hdmi.c are: >> - we should not program HHI_HDMI_CLK_CNTL directly but go through CCF >> (common clock framework) instead (we should already have the driver >> for this in place) >> - we should not program HHI_MEM_PD_REG0 directly but go through the >> genpd/pmdomain framework (we should already have the driver for this >> in place) >> - for the HDMI PHY registers: on Meson8/8b/8m2 (those were 32-bit SoCs >> in case you're not familiar with them, they predate GXBB/GXL/...) I >> wrote a PHY driver (which is already upstream: >> drivers/phy/amlogic/phy-meson8-hdmi-tx.c) as that made most sense to >> me >> >> Then there's meson_venc.c which has two writes to HHI_GCLK_MPEG2. >> I think those should go through CCF instead of directly accessing this register. >> >> Also there's the VDAC registers in meson_encoder_cvbs.c: >> I think Neil suggested at one point to make it a PHY driver. I even >> started working on this (if you're curious: see [0] and [1]). >> DT ABI backwards compatibility is also a concern here. >> >> And finally there's the video clock tree programming in meson_vclk.c. >> My understanding here is that video PLL settings are very sensitive >> and require fine-tuning according to the desired output clock. >> Since it's a bunch of clocks I'd say that direct programming of the >> clock registers should be avoided and we need to go through CCF as >> well. >> But to me those register values are all magic (as I am not aware of >> any documentation that's available to me which describes how to >> determine the correct PLL register values - otherside the standard >> en/m/n/frac/lock and reset bits). >> >> I'm not saying that all of my thoughts are correct and immediately >> need to be put into code. >> Think of them more as an explanation to Jerome's reaction. >> >> I think what we need next is a discussion on how to move forward with >> device-tree ABI for new SoCs. >> Neil, Jerome: I'd like to hear your feedback on this. > I completely agree with your description of the problem, that and > Krzysztof's remark on patch 6. This is not new with this series indeed, > so it does not introduce new problems really but it compounds the > existing ones. > > Addressing those issues, if we want to, will get more difficult as more > support is added for sure. Hi，jerome What are the next steps for "an effort to remove the use HHI syscon" patch set, and what is the schedule? >>> 2 The S4-series secures access to HDMITX DWC and TOP registers, >>> requiring modifications to the driver to handle this new access method. > Regmap buses are made for those cases where the registers are the same, > but accessed differently. There is an example in the patchset referenced by > Martin, to handle GXL and G12 diff. > >> This info should go into patch 1/11 to explain why the g12a compatible >> string cannot be used as fallback. >> >> >> Best regards, >> Martin >> >> >> [0] https://github.com/xdarklight/linux/commit/36e698edc25dc698a0d57b729a7a4587fafc0987 >> [1] https://github.com/xdarklight/linux/commit/824b792fdbd2d3c0b71b21e1105eca0aaad8daa6 > -- > Jerome
On Wed 22 Jan 2025 at 17:50, Ao Xu <ao.xu@amlogic.com> wrote: > On 2025/1/15 1:50, Jerome Brunet wrote: >> [ EXTERNAL EMAIL ] >> >> On Sun 12 Jan 2025 at 23:44, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: >> >>> Hello, >>> >>> On Fri, Jan 10, 2025 at 6:39 AM Ao Xu via B4 Relay >>> <devnull+ao.xu.amlogic.com@kernel.org> wrote: >>>> This patch series adds DRM support for the Amlogic S4-series SoCs. >>>> Compared to the Amlogic G12-series, the S4-series introduces the following changes: >>> Thanks for your patches. With this mail I'll try to summarize my >>> understanding of the situation with the drm/meson driver as I'm not >>> sure how familiar you are with previous discussions. >>> >>>> 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. >>>> As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. >>> Jerome has already commented on patch 3/11 that this mixes in too many >>> IP blocks into one driver. >>> This is not a new situation, the existing code is doing exactly the same. >>> >>> Jerome has previously sent a series which started "an effort to remove >>> the use HHI syscon" [0] from the drm/meson hdmi driver. >>> In the same mail he further notes: "[the patchset] stops short of >>> making any controversial DT changes. To decouple the HDMI >>> phy driver and main DRM driver, allowing the PHY to get hold of its >>> registers, I believe a DT ABI break is inevitable. Ideally the >>> register region of the PHY within the HHI should provided, not the >>> whole HHI. That's pain for another day ..." >>> >>> For now I'm assuming you're familiar with device-tree ABI. >>> If not then please let us know so we can elaborate further on this. >>> >>> My own notes for meson_dw_hdmi.c are: >>> - we should not program HHI_HDMI_CLK_CNTL directly but go through CCF >>> (common clock framework) instead (we should already have the driver >>> for this in place) >>> - we should not program HHI_MEM_PD_REG0 directly but go through the >>> genpd/pmdomain framework (we should already have the driver for this >>> in place) >>> - for the HDMI PHY registers: on Meson8/8b/8m2 (those were 32-bit SoCs >>> in case you're not familiar with them, they predate GXBB/GXL/...) I >>> wrote a PHY driver (which is already upstream: >>> drivers/phy/amlogic/phy-meson8-hdmi-tx.c) as that made most sense to >>> me >>> >>> Then there's meson_venc.c which has two writes to HHI_GCLK_MPEG2. >>> I think those should go through CCF instead of directly accessing this register. >>> >>> Also there's the VDAC registers in meson_encoder_cvbs.c: >>> I think Neil suggested at one point to make it a PHY driver. I even >>> started working on this (if you're curious: see [0] and [1]). >>> DT ABI backwards compatibility is also a concern here. >>> >>> And finally there's the video clock tree programming in meson_vclk.c. >>> My understanding here is that video PLL settings are very sensitive >>> and require fine-tuning according to the desired output clock. >>> Since it's a bunch of clocks I'd say that direct programming of the >>> clock registers should be avoided and we need to go through CCF as >>> well. >>> But to me those register values are all magic (as I am not aware of >>> any documentation that's available to me which describes how to >>> determine the correct PLL register values - otherside the standard >>> en/m/n/frac/lock and reset bits). >>> >>> I'm not saying that all of my thoughts are correct and immediately >>> need to be put into code. >>> Think of them more as an explanation to Jerome's reaction. >>> >>> I think what we need next is a discussion on how to move forward with >>> device-tree ABI for new SoCs. >>> Neil, Jerome: I'd like to hear your feedback on this. >> I completely agree with your description of the problem, that and >> Krzysztof's remark on patch 6. This is not new with this series indeed, >> so it does not introduce new problems really but it compounds the >> existing ones. >> >> Addressing those issues, if we want to, will get more difficult as more >> support is added for sure. > > Hi，jerome > > What are the next steps for "an effort to remove the use HHI syscon" > patch set, and what is the schedule? I have no idea. You should check with Neil whether or not it is something he wants to do and how. If you (or anyone else) want to make a v2 out of the first clean-up I've made [2], you are welcome to. I'm handling other things ATM and I don't expect to get to it any time soon. [2]: https://lore.kernel.org/linux-amlogic/20240730125023.710237-1-jbrunet@baylibre.com/ > >>>> 2 The S4-series secures access to HDMITX DWC and TOP registers, >>>> requiring modifications to the driver to handle this new access method. >> Regmap buses are made for those cases where the registers are the same, >> but accessed differently. There is an example in the patchset referenced by >> Martin, to handle GXL and G12 diff. >> >>> This info should go into patch 1/11 to explain why the g12a compatible >>> string cannot be used as fallback. >>> >>> >>> Best regards, >>> Martin >>> >>> >>> [0] https://github.com/xdarklight/linux/commit/36e698edc25dc698a0d57b729a7a4587fafc0987 >>> [1] >>> https://github.com/xdarklight/linux/commit/824b792fdbd2d3c0b71b21e1105eca0aaad8daa6 >> -- >> Jerome
This patch series adds DRM support for the Amlogic S4-series SoCs. Compared to the Amlogic G12-series, the S4-series introduces the following changes: 1 The S4-series splits the HIU into three separate components: `sys_ctrl`, `pwr_ctrl`, and `clk_ctrl`. As a result, VENC and VCLK drivers are updated with S4-specific compatible strings to accommodate these changes. 2 The S4-series secures access to HDMITX DWC and TOP registers, requiring modifications to the driver to handle this new access method. 3 The register addresses for the video1 and video2 planes have been updated in the S4 hardware, and the DRM driver has been adapted accordingly. 4 The OSD, VIU, and VPP components remain unchanged and are consistent with the G12-series. Signed-off-by: Ao Xu <ao.xu@amlogic.com> --- Ao Xu (11): dt-bindings: display: meson-dw-hdmi: Add compatible for S4 HDMI controller dt-bindings: display: meson-vpu: Add compatible for S4 display controller drm: meson: add S4 compatible for DRM driver drm: meson: add primary and overlay plane support for S4 drm: meson: update VIU and VPP support for S4 drm: meson: add meson_dw_hdmi support for S4 drm: meson: change api call parameter drm: meson: add hdmitx vmode timing support for S4 drm: meson: add vpu clk setting for S4 drm: meson: add CVBS support for S4 arm64: dts: amlogic: s4: add DRM support [1/1] .../bindings/display/amlogic,meson-dw-hdmi.yaml | 1 + .../bindings/display/amlogic,meson-vpu.yaml | 48 +- .../boot/dts/amlogic/meson-s4-s805x2-aq222.dts | 39 + arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 121 +++ drivers/gpu/drm/meson/meson_crtc.c | 90 +- drivers/gpu/drm/meson/meson_drv.c | 127 ++- drivers/gpu/drm/meson/meson_drv.h | 6 + drivers/gpu/drm/meson/meson_dw_hdmi.c | 244 ++++- drivers/gpu/drm/meson/meson_dw_hdmi.h | 126 +++ drivers/gpu/drm/meson/meson_encoder_cvbs.c | 10 + drivers/gpu/drm/meson/meson_encoder_hdmi.c | 19 +- drivers/gpu/drm/meson/meson_overlay.c | 7 +- drivers/gpu/drm/meson/meson_plane.c | 24 +- drivers/gpu/drm/meson/meson_registers.h | 17 + drivers/gpu/drm/meson/meson_vclk.c | 1018 ++++++++++++++------ drivers/gpu/drm/meson/meson_venc.c | 346 ++++++- drivers/gpu/drm/meson/meson_venc.h | 4 +- drivers/gpu/drm/meson/meson_viu.c | 9 +- drivers/gpu/drm/meson/meson_vpp.c | 12 +- 19 files changed, 1865 insertions(+), 403 deletions(-) --- base-commit: 6ecd20965bdc21b265a0671ccf36d9ad8043f5ab change-id: 20250110-drm-s4-c96c88be52e4 Best regards,