On 06/11/2015 11:01 PM, Gustavo Padovan wrote: > Hi Joonyoung, > > 2015-06-11 Joonyoung Shim <jy0922.shim at samsung.com>: > >> On 06/10/2015 10:36 PM, Gustavo Padovan wrote: >>> Hi Marek, >>> >>> 2015-06-10 Marek Szyprowski <m.szyprowski at samsung.com>: >>> >>>> Hello, >>>> >>>> On 2015-06-01 17:04, Gustavo Padovan wrote: >>>>> From: Gustavo Padovan <gustavo.padovan at 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 at 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 at 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? >>> >>> It should not, you are right. This is actually the correct fix. Atomic >>> modesetting should not do chained calls, e.g., crtc_disable calling >>> plane_disable. This change was already in my plan actually, but as I had >>> IOMMU disabled I didn't see it here, and I didn't create this patch yet. >>> >> >> Right, but it needs that crtc_disable calls plane_disable in exynos >> driver internally. Because crtc is disabled before plane is disabled, >> it means plane_disable just returns without any register changes, >> then we cannot be sure setting register to disable plane when crtc is >> disable. >> >> I think a solution is enough only to eliminate calling xxx_resume >> function in exynos driver function when enable plane, we can remove it >> because it's called to enable plane from drm atomic modeset framework. > > I think this should work. We really need to disable the planes before > the crtc is disabled, for FIMD for example, the overlay planes are > removed from the screen if we remove the _suspend call. > > I'll sent a new patch to remove only the resume part from all our crtc > drivers. >
I sent patchset related, is it same with which you try? Thanks.