Re: [PATCH v10 00/17] drm/exynos: atomic modesetting support

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hello,

On 2015-06-10 12:59, Inki Dae wrote:
> Hi Marek,
>
> On 2015년 06월 10일 19:03, Marek Szyprowski wrote:
>> Hello,
>>
>> On 2015-06-01 17:04, Gustavo Padovan wrote:
>>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>>
>>> Hi,
>>>
>>> Here goes the full support for atomic modesetting on exynos. I've
>>> split the patches in the various phases of atomic support.
>> Thanks for this patchses, however I've noticed a problem after applying
>> them.
>> The issue gets revealed when support for IOMMU is enabled. I've did my
>> tests
>> with Exynos HDMI driver on Odroid U3 board.
>>
>> To demonstrated the issue I've added following additional debug in the
>> exynos
>> mixer driver in mixer_graph_buffer() function:
>> pr_info("dma addr %pad plane->src_width %d plane->src_height %d\n",
>> &plane->dma_addr[0], plane->src_width, plane->src_height);
>>
>> Before applying patches setting 640x480 mode and getting back to 1920x1080
>> console generates following log:
>>
>> # modetest -M exynos -s 23:640x480
>> setting mode 640x480-60Hz@XR24 on connectors 23, crtc 21
>> [ 3860.617151] dma 0xbc500000 plane->src_width 640 plane->src_height 480
>> ^C
>> [ 3870.555232] dma 0xbbd00000 plane->src_width 1920 plane->src_height 1080
>> [ 3870.565696] dma 0xbbd00000 plane->src_width 1920 plane->src_height 1080
>>
>> After applying atomic modesetting patchset:
>> # modetest -M exynos -s 24:640x480
>> [  142.540122] dma 0xbbd00000 plane->src_width 1920 plane->src_height 1080
>> [  142.550726] dma 0xbbd00000 plane->src_width 1920 plane->src_height 1080
>> setting mode 640x480-60Hz@XR24 on connectors 24, crtc 22
>> [  142.643672] dma 0xbc500000 plane->src_width 1920 plane->src_height 1080
>> [  142.759982] dma 0xbc500000 plane->src_width 640 plane->src_height 480
>> ^C
>> [  154.986040] dma 0xbbd00000 plane->src_width 1920 plane->src_height 1080
>>
>> As you can see from the above log, mixer_graph_buffer function is called
>> several times. 0xbbd00000 is the DMA address of the 1920x1080 framebuffer
>> and 0xbc500000 is the DMA address of the allocated 640x480 buffer.
>> mixer_graph_buffer() is first called with the new DMA address of the
>> framebuffer, but with the old mode parameters (1920x1080 size) and then
>> in the next call it updates the plane parameters to the correct values
>> (size changed to 640x480). When IOMMU is not used, this can be easily
>> missed, but after enabling IOMMU support, any DMA access to unallocated
>> address causes IOMMU PAGE FAULT. Here it will happen after changing DMA
>> address of the buffer without changing the size.
>>
>> A quick workaround to resolve this multiple calls to mixer_graph_buffer()
>> with partially updated mode values is to remove calls to
>> mixer_window_suspend/mixer_window_resume from mixer_disable and
>> mixer_disable functions, but I expect that this is not the right
>> approach.
>>
>> Probably the same problem can be observed with Exynos FIMD driver.
>>
>> Gustavo: could you check if mixer_enable functions should really
>> call mixer_window_resume function, which in turn calls mixer_win_commit,
>> which calls mixer_graph_buffer with partially updated display buffer
>> data?
> Marek, can you share how other people can test the atomic feature with
> iommu?
>
> I should have merged below several patches and added device tree
> relevant codes to test iommu.
>
> 1. Merged iommu support patches for Exynos SoC below iommu exynos tree,
>
> https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=arm/exynos
>
> 2. Merged below patch,
>      [PATCH v7 24/25] ARM: DMA-mapping: add
> support for creating reserved mappings in iova space
>
> 3. Added device node relevant codes - I tested Exynos drm on trats2
> board based on Exynos4412 SoC - like below,
> in exynos4.dtsi file:
> fimd: fimd@11c00000 {
>                   ...
>                 iommus = <&sysmmu_fimd0>;
>                   ...
>
> sysmmu_fimd0: sysmmu@11E20000 {
>          compatible = "samsung,exynos-sysmmu";
>          reg = <0x11E20000 0x1000>;
>          interrupt-parent = <&combiner>;
>          interrupts = <5 2>;
>          clock-names = "sysmmu", "master";
>          clocks = <&clock CLK_SMMU_FIMD0>, <&clock CLK_FIMD0>;
>           power-domains = <&pd_lcd0>;
>          #iommu-cells = <0>;
> };
>
> in exynos4412-trats2.dts file:
> fimd@11c00000 {
>                  status = "okay";
>                  iommu-reserved-mapping = <0x40000000 0x40000000 0x40000000>;
> };
>
> Is that all? You would need to share exact guide about iommu enabling to
> other people so that they can test atomic feature with iommu.

Right, the above should be enough. For convenience I've prepared a 
branch with all needed patches:
https://git.linaro.org/people/marek.szyprowski/linux-srpol.git 
v4.1-exynos-drm-iommu

I've included following branches:
https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/log/?h=exynos-drm/for-next
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=v4.2-next/dt-samsung-4th
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=v4.2-next/mach-samsung
https://git.kernel.org/cgit/linux/kernel/git/joro/iommu.git/log/?h=arm/exynos

Then I've applied my Exynos DRM patches 
(http://www.spinics.net/lists/linux-samsung-soc/msg45114.html)
as well as rebased patch 24/25 and 25/25 from my initial v7 IOMMU series
(http://www.spinics.net/lists/linux-samsung-soc/msg44652.html).

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland