| Message ID | 1506670374-15689-1-git-send-email-m.szyprowski@samsung.com |
|---|---|
| Headers | show |
| Series | Exynos DRM: rewrite IPP subsystem and userspace API | expand |
Hello Marek, I just tested this series, and I noticed a lot of these lines: > exynos-sysmmu 11a40000.sysmmu: restoring state > exynos-sysmmu 11a40000.sysmmu: saving state I guess it would make sense to enable autosuspend for runtime PM in each of the hw drivers. I've just send a patch that does it for the FIMC (the only hw I can test this on). This improves frame time stability for me. With best wishes, Tobias Marek Szyprowski wrote: > Dear all, > > This patchset performs complete rewrite of Exynos DRM IPP subsystem and > its userspace API. > > Why such rewrite is needed? Exynos DRM IPP API is over-engineered in > general, but not really extensible on the other side. It is also buggy, > with significant design flaws: > - Userspace API covers memory-2-memory picture operations together with > CRTC writeback and duplicating features, which belongs to video plane. > - Lack of support of the all required image formats (for example NV12 > Samsung-tiled cannot be used due to lack of pixel format modifier > support). > - Userspace API designed only to mimic hardware behaviour, not easy to > understand. > - Lack of proper input validation in the core, drivers also didn't do that > correctly, so it was possible to set incorrect parameters and easil > trigger IOMMU fault or memory trash. > - Drivers were partially disfunctional or supported only a subset of modes. > > Due to the above limitations and issues the Exynos DRM IPP API was not > used by any of the open-source projects. I assume that it is safe to remove > this broken API without any damage to open-source community. All remaining > users (mainly Tizen project related) will be updated to the new version. > > This patchset changes Exynos DRM IPP subsystem to something useful. The > userspace API is much simpler, state-less and easy to understand. Also > the code of the core and driver is significantly smaller and easier to > understand. > > Patches were tested on Exynos4412 based Odroid U3 and Exynos5422 > Odroid XU3 boards, on top of Linux next-20170928 kernel. > > Best regards > Marek Szyprowski > Samsung R&D Institute Poland > > > Changelog: > > v2: > - fixed minor issues pointed by other developers: > * fixed possible null pointer dereferrence (Tobias) > * changed limits_size to limits_count (Tobias) > * renamed struct exynos_drm_ipp_format to drm_exynos_ipp_format (Andrzej) > * added proper return value from exynos_drm_ipp_get_res_ioctl when no IPP > driver is present (Andrzej) > * properly aligned all uapi structures to be 32/64 bit safe (Emil) > * properly initialize all strucutres > - added new Exynos Scaler driver from Andrzej Pietrasiewicz > > v1: https://www.spinics.net/lists/linux-samsung-soc/msg60492.html > - initial version of IPP v2 > > My previous works in this area: > > "[RFC v2 0/2] Exynos DRM: add Picture Processor extension" > https://www.spinics.net/lists/dri-devel/msg140669.html > - removed usage of DRM objects and properties - replaced them with simple > list of parameters with predefined IDs > > "[RFC 0/4] Exynos DRM: add Picture Processor extension" > https://www.spinics.net/lists/linux-samsung-soc/msg59323.html > - moved this feature from DRM core to Exynos DRM driver > - changed name from framebuffer processor to picture processor > - simplified code to cover only things needed by Exynos drivers > - implemented simple fifo task scheduler > - cleaned up rotator driver conversion (removed IPP remainings) > > "[RFC 0/2] New feature: Framebuffer processors" > https://www.spinics.net/lists/linux-samsung-soc/msg54810.html > - generic approach implemented in DRM core, rejected > > > Patch summary: > > Andrzej Pietrasiewicz (3): > drm/exynos: Add driver for Exynos Scaler module > drivers: clk: samsung: Fix m2m scaler clock on Exynos542x > ARM: dts: exynos: Add mem-2-mem Scaler devices > > Marek Szyprowski (6): > drm/exynos: ipp: Remove Exynos DRM IPP subsystem > drm/exynos: ipp: Add IPP v2 framework > drm/exynos: rotator: Convert driver to IPP v2 core API > drm/exynos: gsc: Convert driver to IPP v2 core API > drm/exynos: Add generic support for devices shared with V4L2 subsystem > drm/exynos: fimc: Convert driver to IPP v2 core API > > .../devicetree/bindings/gpu/samsung-scaler.txt | 25 + > arch/arm/boot/dts/exynos5420.dtsi | 35 + > drivers/clk/samsung/clk-exynos5420.c | 2 +- > drivers/gpu/drm/exynos/Kconfig | 18 +- > drivers/gpu/drm/exynos/Makefile | 1 + > drivers/gpu/drm/exynos/exynos_drm_drv.c | 36 +- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 5 +- > drivers/gpu/drm/exynos/exynos_drm_fimc.c | 893 +++----- > drivers/gpu/drm/exynos/exynos_drm_fimc.h | 23 - > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 853 ++------ > drivers/gpu/drm/exynos/exynos_drm_gsc.h | 24 - > drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2240 ++++++-------------- > drivers/gpu/drm/exynos/exynos_drm_ipp.h | 357 ++-- > drivers/gpu/drm/exynos/exynos_drm_rotator.c | 735 ++----- > drivers/gpu/drm/exynos/exynos_drm_rotator.h | 19 - > drivers/gpu/drm/exynos/exynos_drm_scaler.c | 675 ++++++ > drivers/gpu/drm/exynos/regs-scaler.h | 426 ++++ > include/uapi/drm/exynos_drm.h | 326 +-- > 18 files changed, 2818 insertions(+), 3875 deletions(-) > create mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.txt > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.h > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_gsc.h > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_rotator.h > create mode 100644 drivers/gpu/drm/exynos/exynos_drm_scaler.c > create mode 100644 drivers/gpu/drm/exynos/regs-scaler.h > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tobias, On 2017-09-29 15:24, Tobias Jakobi wrote: > Hello Marek, > > I just tested this series, and I noticed a lot of these lines: >> exynos-sysmmu 11a40000.sysmmu: restoring state >> exynos-sysmmu 11a40000.sysmmu: saving state > I guess it would make sense to enable autosuspend for runtime PM in each of the > hw drivers. > > I've just send a patch that does it for the FIMC (the only hw I can test this > on). This improves frame time stability for me. Right, autosuspend definitely makes sense in case of IPP drivers. I will update all to use it. Thanks for testing this stuff and pointing this issue! > ... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello everyone, I have finished some first (working) version of my mpv video backend for IPPv2. You can find the tree here: https://github.com/tobiasjakobi/mpv I've also created some RFC pull request upstream, to get some input on the current patches: https://github.com/mpv-player/mpv/pull/4986 As you can see, a lot is currently missing, but at least the video presentation (for YUV420) already works and shows good performance. I have also cleaned up my ippv2 libdrm branch. Now it should just contain the IPPv2 bits: https://github.com/tobiasjakobi/libdrm/tree/ippv2 With best wishes, Tobias Marek Szyprowski wrote: > Dear all, > > This patchset performs complete rewrite of Exynos DRM IPP subsystem and > its userspace API. > > Why such rewrite is needed? Exynos DRM IPP API is over-engineered in > general, but not really extensible on the other side. It is also buggy, > with significant design flaws: > - Userspace API covers memory-2-memory picture operations together with > CRTC writeback and duplicating features, which belongs to video plane. > - Lack of support of the all required image formats (for example NV12 > Samsung-tiled cannot be used due to lack of pixel format modifier > support). > - Userspace API designed only to mimic hardware behaviour, not easy to > understand. > - Lack of proper input validation in the core, drivers also didn't do that > correctly, so it was possible to set incorrect parameters and easil > trigger IOMMU fault or memory trash. > - Drivers were partially disfunctional or supported only a subset of modes. > > Due to the above limitations and issues the Exynos DRM IPP API was not > used by any of the open-source projects. I assume that it is safe to remove > this broken API without any damage to open-source community. All remaining > users (mainly Tizen project related) will be updated to the new version. > > This patchset changes Exynos DRM IPP subsystem to something useful. The > userspace API is much simpler, state-less and easy to understand. Also > the code of the core and driver is significantly smaller and easier to > understand. > > Patches were tested on Exynos4412 based Odroid U3 and Exynos5422 > Odroid XU3 boards, on top of Linux next-20170928 kernel. > > Best regards > Marek Szyprowski > Samsung R&D Institute Poland > > > Changelog: > > v2: > - fixed minor issues pointed by other developers: > * fixed possible null pointer dereferrence (Tobias) > * changed limits_size to limits_count (Tobias) > * renamed struct exynos_drm_ipp_format to drm_exynos_ipp_format (Andrzej) > * added proper return value from exynos_drm_ipp_get_res_ioctl when no IPP > driver is present (Andrzej) > * properly aligned all uapi structures to be 32/64 bit safe (Emil) > * properly initialize all strucutres > - added new Exynos Scaler driver from Andrzej Pietrasiewicz > > v1: https://www.spinics.net/lists/linux-samsung-soc/msg60492.html > - initial version of IPP v2 > > My previous works in this area: > > "[RFC v2 0/2] Exynos DRM: add Picture Processor extension" > https://www.spinics.net/lists/dri-devel/msg140669.html > - removed usage of DRM objects and properties - replaced them with simple > list of parameters with predefined IDs > > "[RFC 0/4] Exynos DRM: add Picture Processor extension" > https://www.spinics.net/lists/linux-samsung-soc/msg59323.html > - moved this feature from DRM core to Exynos DRM driver > - changed name from framebuffer processor to picture processor > - simplified code to cover only things needed by Exynos drivers > - implemented simple fifo task scheduler > - cleaned up rotator driver conversion (removed IPP remainings) > > "[RFC 0/2] New feature: Framebuffer processors" > https://www.spinics.net/lists/linux-samsung-soc/msg54810.html > - generic approach implemented in DRM core, rejected > > > Patch summary: > > Andrzej Pietrasiewicz (3): > drm/exynos: Add driver for Exynos Scaler module > drivers: clk: samsung: Fix m2m scaler clock on Exynos542x > ARM: dts: exynos: Add mem-2-mem Scaler devices > > Marek Szyprowski (6): > drm/exynos: ipp: Remove Exynos DRM IPP subsystem > drm/exynos: ipp: Add IPP v2 framework > drm/exynos: rotator: Convert driver to IPP v2 core API > drm/exynos: gsc: Convert driver to IPP v2 core API > drm/exynos: Add generic support for devices shared with V4L2 subsystem > drm/exynos: fimc: Convert driver to IPP v2 core API > > .../devicetree/bindings/gpu/samsung-scaler.txt | 25 + > arch/arm/boot/dts/exynos5420.dtsi | 35 + > drivers/clk/samsung/clk-exynos5420.c | 2 +- > drivers/gpu/drm/exynos/Kconfig | 18 +- > drivers/gpu/drm/exynos/Makefile | 1 + > drivers/gpu/drm/exynos/exynos_drm_drv.c | 36 +- > drivers/gpu/drm/exynos/exynos_drm_drv.h | 5 +- > drivers/gpu/drm/exynos/exynos_drm_fimc.c | 893 +++----- > drivers/gpu/drm/exynos/exynos_drm_fimc.h | 23 - > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 853 ++------ > drivers/gpu/drm/exynos/exynos_drm_gsc.h | 24 - > drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2240 ++++++-------------- > drivers/gpu/drm/exynos/exynos_drm_ipp.h | 357 ++-- > drivers/gpu/drm/exynos/exynos_drm_rotator.c | 735 ++----- > drivers/gpu/drm/exynos/exynos_drm_rotator.h | 19 - > drivers/gpu/drm/exynos/exynos_drm_scaler.c | 675 ++++++ > drivers/gpu/drm/exynos/regs-scaler.h | 426 ++++ > include/uapi/drm/exynos_drm.h | 326 +-- > 18 files changed, 2818 insertions(+), 3875 deletions(-) > create mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.txt > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.h > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_gsc.h > delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_rotator.h > create mode 100644 drivers/gpu/drm/exynos/exynos_drm_scaler.c > create mode 100644 drivers/gpu/drm/exynos/regs-scaler.h > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear all, This patchset performs complete rewrite of Exynos DRM IPP subsystem and its userspace API. Why such rewrite is needed? Exynos DRM IPP API is over-engineered in general, but not really extensible on the other side. It is also buggy, with significant design flaws: - Userspace API covers memory-2-memory picture operations together with CRTC writeback and duplicating features, which belongs to video plane. - Lack of support of the all required image formats (for example NV12 Samsung-tiled cannot be used due to lack of pixel format modifier support). - Userspace API designed only to mimic hardware behaviour, not easy to understand. - Lack of proper input validation in the core, drivers also didn't do that correctly, so it was possible to set incorrect parameters and easil trigger IOMMU fault or memory trash. - Drivers were partially disfunctional or supported only a subset of modes. Due to the above limitations and issues the Exynos DRM IPP API was not used by any of the open-source projects. I assume that it is safe to remove this broken API without any damage to open-source community. All remaining users (mainly Tizen project related) will be updated to the new version. This patchset changes Exynos DRM IPP subsystem to something useful. The userspace API is much simpler, state-less and easy to understand. Also the code of the core and driver is significantly smaller and easier to understand. Patches were tested on Exynos4412 based Odroid U3 and Exynos5422 Odroid XU3 boards, on top of Linux next-20170928 kernel. Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v2: - fixed minor issues pointed by other developers: * fixed possible null pointer dereferrence (Tobias) * changed limits_size to limits_count (Tobias) * renamed struct exynos_drm_ipp_format to drm_exynos_ipp_format (Andrzej) * added proper return value from exynos_drm_ipp_get_res_ioctl when no IPP driver is present (Andrzej) * properly aligned all uapi structures to be 32/64 bit safe (Emil) * properly initialize all strucutres - added new Exynos Scaler driver from Andrzej Pietrasiewicz v1: https://www.spinics.net/lists/linux-samsung-soc/msg60492.html - initial version of IPP v2 My previous works in this area: "[RFC v2 0/2] Exynos DRM: add Picture Processor extension" https://www.spinics.net/lists/dri-devel/msg140669.html - removed usage of DRM objects and properties - replaced them with simple list of parameters with predefined IDs "[RFC 0/4] Exynos DRM: add Picture Processor extension" https://www.spinics.net/lists/linux-samsung-soc/msg59323.html - moved this feature from DRM core to Exynos DRM driver - changed name from framebuffer processor to picture processor - simplified code to cover only things needed by Exynos drivers - implemented simple fifo task scheduler - cleaned up rotator driver conversion (removed IPP remainings) "[RFC 0/2] New feature: Framebuffer processors" https://www.spinics.net/lists/linux-samsung-soc/msg54810.html - generic approach implemented in DRM core, rejected Patch summary: Andrzej Pietrasiewicz (3): drm/exynos: Add driver for Exynos Scaler module drivers: clk: samsung: Fix m2m scaler clock on Exynos542x ARM: dts: exynos: Add mem-2-mem Scaler devices Marek Szyprowski (6): drm/exynos: ipp: Remove Exynos DRM IPP subsystem drm/exynos: ipp: Add IPP v2 framework drm/exynos: rotator: Convert driver to IPP v2 core API drm/exynos: gsc: Convert driver to IPP v2 core API drm/exynos: Add generic support for devices shared with V4L2 subsystem drm/exynos: fimc: Convert driver to IPP v2 core API .../devicetree/bindings/gpu/samsung-scaler.txt | 25 + arch/arm/boot/dts/exynos5420.dtsi | 35 + drivers/clk/samsung/clk-exynos5420.c | 2 +- drivers/gpu/drm/exynos/Kconfig | 18 +- drivers/gpu/drm/exynos/Makefile | 1 + drivers/gpu/drm/exynos/exynos_drm_drv.c | 36 +- drivers/gpu/drm/exynos/exynos_drm_drv.h | 5 +- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 893 +++----- drivers/gpu/drm/exynos/exynos_drm_fimc.h | 23 - drivers/gpu/drm/exynos/exynos_drm_gsc.c | 853 ++------ drivers/gpu/drm/exynos/exynos_drm_gsc.h | 24 - drivers/gpu/drm/exynos/exynos_drm_ipp.c | 2240 ++++++-------------- drivers/gpu/drm/exynos/exynos_drm_ipp.h | 357 ++-- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 735 ++----- drivers/gpu/drm/exynos/exynos_drm_rotator.h | 19 - drivers/gpu/drm/exynos/exynos_drm_scaler.c | 675 ++++++ drivers/gpu/drm/exynos/regs-scaler.h | 426 ++++ include/uapi/drm/exynos_drm.h | 326 +-- 18 files changed, 2818 insertions(+), 3875 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.txt delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_fimc.h delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_gsc.h delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_rotator.h create mode 100644 drivers/gpu/drm/exynos/exynos_drm_scaler.c create mode 100644 drivers/gpu/drm/exynos/regs-scaler.h -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html