[PATCH v10 00/17] drm/exynos: atomic modesetting support
On 2015ë 06ì 17ì¼ 23:33, Gustavo Padovan wrote: > Hi Inki, > > 2015-06-17 Inki Dae : > >> Hi Gustavo, >> >> On 2015ë 06ì 17ì¼ 05:35, Gustavo Padovan wrote: >>> HI Inki, >>> >>> 2015-06-15 Inki Dae : >>> Hi Gustavo, On 2015ë 06ì 02ì¼ 00:04, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > Here goes the full support for atomic modesetting on exynos. I've > split the patches in the various phases of atomic support. > > v2: fixes comments by Joonyoung > - remove unused var in patch 09 > - use ->disable instead of outdated ->dpms in hdmi code > - remove WARN_ON from crtc enable/disable > > v3: fixes comment by Joonyoung > - move the removal of drm_helper_disable_unused_functions() to > separated patch > > v4: add patches that remove unnecessary calls to disable_plane() > > v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) > > v6: rebase on latest exynos_drm_next > > v7: fix comments by Joonyoung > - fix two checkpatch errors > - remove extra crtc->commit() call > - check for null fb on exynos_check_plane() > > v8: fix comments by Joonyoung > - fix merge error > - move drm_crtc_vblank_get to the commit that introduces atomic > pageflip > - remove .prepare() in the apropiated patch > - add a new patch to move exynos_drm_crtc_disable() > > v9: * fix comments by Joonyoung > - also remove encoder .prepare() > - do not shift exynos_update_plane() parameters > - remove unused .mode_set() and .mode_set_base() > * add specific exynos .atomic_commit() > * add split of exynos_crtc->ops->dpms() into enable() and disable() > * add other atomic clean ups > > v10: * fix comments by Joonyoung > - add more comments on exynos_atomic_commit() > - make exynos_crtc's .enable and .disable void I found out one issue that refresh rate gets low with display extension mode test. I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here is how to test it, #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 freq: 20.00Hz freq: 20.00Hz As you can see, refresh rate is 20. Below is the result without atomic patch series, # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 freq: 60.18Hz freq: 49.09Hz I can see 60Hz for FIMD and 49Hz for vidi. >>> >>> I gave this a try and figured out that this might be a vidi specific >>> problem. If I try VIDI and FIMD I get the same results as you but with >>> Mixer and FIMD(the setup I actually use to test) everything works fine. >> >> Hm... Did Mixer and FIMD combination really work correctly with >> extension mode? > > Yes. > > collabora at singularity:~$ modetest -M exynos -s 31:1920x1080 -v -s > 25:1366x768 > setting mode 1920x1080-60Hz at XR24 on connectors 31, crtc 29 > setting mode 1366x768-60Hz at XR24 on connectors 25, crtc 23 > freq: 55.82Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz Your patch series had one bug that the vblank operation was duplicated. As you may know, the page flip operation already contains vblank functionality but your patch made atomic_commit callback wait for the completion of vblank again. See the below patch I posted, [PATCH] drm/exynos: do not wait for vblank at atomic operation That was why I asked for that mixer and fimd combination worked correctly with extension mode. Don't you think above frequency is strange? The frequency should be 60Hz, not 55Hz. We would need to enhance the atomic feature for Exynos drm later. Current atomic feature doesn't really do atomic operation, and is required for more cleanups. Anyway, I have already merged it and will have a pull-request soon. Thanks for your contributions, Inki Dae > >> >>> So somehow my patches caused a regression on vidi that I'm still >>> ivestigating. >> >> I think this issue is because page flip operations are performed in >> atomic: if I see correctly, when page flip is requested by modetest, the >> call flow is as follows, >> >> drm_atomic_helper_page_flip >> drm_atomic_async_commit >> exynos_atomic_commit >> ... >> drm_atomic_helper_wait_for_vblanks >> ... >> >> However, the function, drm_a
[PATCH v10 00/17] drm/exynos: atomic modesetting support
On 2015ë 06ì 17ì¼ 23:33, Gustavo Padovan wrote: > Hi Inki, > > 2015-06-17 Inki Dae : > >> Hi Gustavo, >> >> On 2015ë 06ì 17ì¼ 05:35, Gustavo Padovan wrote: >>> HI Inki, >>> >>> 2015-06-15 Inki Dae : >>> Hi Gustavo, On 2015ë 06ì 02ì¼ 00:04, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > Here goes the full support for atomic modesetting on exynos. I've > split the patches in the various phases of atomic support. > > v2: fixes comments by Joonyoung > - remove unused var in patch 09 > - use ->disable instead of outdated ->dpms in hdmi code > - remove WARN_ON from crtc enable/disable > > v3: fixes comment by Joonyoung > - move the removal of drm_helper_disable_unused_functions() to > separated patch > > v4: add patches that remove unnecessary calls to disable_plane() > > v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) > > v6: rebase on latest exynos_drm_next > > v7: fix comments by Joonyoung > - fix two checkpatch errors > - remove extra crtc->commit() call > - check for null fb on exynos_check_plane() > > v8: fix comments by Joonyoung > - fix merge error > - move drm_crtc_vblank_get to the commit that introduces atomic > pageflip > - remove .prepare() in the apropiated patch > - add a new patch to move exynos_drm_crtc_disable() > > v9: * fix comments by Joonyoung > - also remove encoder .prepare() > - do not shift exynos_update_plane() parameters > - remove unused .mode_set() and .mode_set_base() > * add specific exynos .atomic_commit() > * add split of exynos_crtc->ops->dpms() into enable() and disable() > * add other atomic clean ups > > v10: * fix comments by Joonyoung > - add more comments on exynos_atomic_commit() > - make exynos_crtc's .enable and .disable void I found out one issue that refresh rate gets low with display extension mode test. I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here is how to test it, #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 freq: 20.00Hz freq: 20.00Hz As you can see, refresh rate is 20. Below is the result without atomic patch series, # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 freq: 60.18Hz freq: 49.09Hz I can see 60Hz for FIMD and 49Hz for vidi. >>> >>> I gave this a try and figured out that this might be a vidi specific >>> problem. If I try VIDI and FIMD I get the same results as you but with >>> Mixer and FIMD(the setup I actually use to test) everything works fine. >> >> Hm... Did Mixer and FIMD combination really work correctly with >> extension mode? > > Yes. > > collabora at singularity:~$ modetest -M exynos -s 31:1920x1080 -v -s > 25:1366x768 > setting mode 1920x1080-60Hz at XR24 on connectors 31, crtc 29 > setting mode 1366x768-60Hz at XR24 on connectors 25, crtc 23 > freq: 55.82Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > freq: 55.30Hz > freq: 55.38Hz > >> >>> So somehow my patches caused a regression on vidi that I'm still >>> ivestigating. >> >> I think this issue is because page flip operations are performed in >> atomic: if I see correctly, when page flip is requested by modetest, the >> call flow is as follows, >> >> drm_atomic_helper_page_flip >> drm_atomic_async_commit >> exynos_atomic_commit >> ... >> drm_atomic_helper_wait_for_vblanks >> ... >> >> However, the function, drm_atomic_helper_wait_for_vblanks called by >> exynos_atomic_commit, waits for the vblank completion of each crtc >> driver . See wait_event_timeout function call in >> drm_atomic_helper_wait_for_vblanks function. >> >> This means that a page flip request of a crtc driver cannot be performed >> until the vblank completion of another crtc driver. I think you should >> have implemented exynos_atomic_commit function asynchronously, not >> synchronously like you did. So it seems that this function should be >> re-implemented. > > I have a patch for it already. I'll resend a v2 of my last series. Can you post it now? Thanks, Inki Dae > >> >> If my analysis is correct and you cannot post the change set within this >> week, I'm not sure whether the atomic patch series should
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hi Gustavo, On 2015ë 06ì 17ì¼ 05:35, Gustavo Padovan wrote: > HI Inki, > > 2015-06-15 Inki Dae : > >> Hi Gustavo, >> >> On 2015ë 06ì 02ì¼ 00:04, Gustavo Padovan wrote: >>> From: Gustavo Padovan >>> >>> Hi, >>> >>> Here goes the full support for atomic modesetting on exynos. I've >>> split the patches in the various phases of atomic support. >>> >>> v2: fixes comments by Joonyoung >>> - remove unused var in patch 09 >>> - use ->disable instead of outdated ->dpms in hdmi code >>> - remove WARN_ON from crtc enable/disable >>> >>> v3: fixes comment by Joonyoung >>> - move the removal of drm_helper_disable_unused_functions() to >>> separated patch >>> >>> v4: add patches that remove unnecessary calls to disable_plane() >>> >>> v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) >>> >>> v6: rebase on latest exynos_drm_next >>> >>> v7: fix comments by Joonyoung >>> - fix two checkpatch errors >>> - remove extra crtc->commit() call >>> - check for null fb on exynos_check_plane() >>> >>> v8: fix comments by Joonyoung >>> - fix merge error >>> - move drm_crtc_vblank_get to the commit that introduces atomic >>> pageflip >>> - remove .prepare() in the apropiated patch >>> - add a new patch to move exynos_drm_crtc_disable() >>> >>> v9: * fix comments by Joonyoung >>> - also remove encoder .prepare() >>> - do not shift exynos_update_plane() parameters >>> - remove unused .mode_set() and .mode_set_base() >>> * add specific exynos .atomic_commit() >>> * add split of exynos_crtc->ops->dpms() into enable() and disable() >>> * add other atomic clean ups >>> >>> v10: * fix comments by Joonyoung >>> - add more comments on exynos_atomic_commit() >>> - make exynos_crtc's .enable and .disable void >> >> I found out one issue that refresh rate gets low with display extension >> mode test. >> >> I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here >> is how to test it, >> >> #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection >> # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 >> setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 >> setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 >> freq: 20.00Hz >> freq: 20.00Hz >> >> As you can see, refresh rate is 20. >> >> Below is the result without atomic patch series, >> # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 >> setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 >> setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 >> freq: 60.18Hz >> freq: 49.09Hz >> >> I can see 60Hz for FIMD and 49Hz for vidi. > > I gave this a try and figured out that this might be a vidi specific > problem. If I try VIDI and FIMD I get the same results as you but with > Mixer and FIMD(the setup I actually use to test) everything works fine. Hm... Did Mixer and FIMD combination really work correctly with extension mode? > So somehow my patches caused a regression on vidi that I'm still > ivestigating. I think this issue is because page flip operations are performed in atomic: if I see correctly, when page flip is requested by modetest, the call flow is as follows, drm_atomic_helper_page_flip drm_atomic_async_commit exynos_atomic_commit ... drm_atomic_helper_wait_for_vblanks ... However, the function, drm_atomic_helper_wait_for_vblanks called by exynos_atomic_commit, waits for the vblank completion of each crtc driver . See wait_event_timeout function call in drm_atomic_helper_wait_for_vblanks function. This means that a page flip request of a crtc driver cannot be performed until the vblank completion of another crtc driver. I think you should have implemented exynos_atomic_commit function asynchronously, not synchronously like you did. So it seems that this function should be re-implemented. If my analysis is correct and you cannot post the change set within this week, I'm not sure whether the atomic patch series should go to mainline. Anyway, I will decide it and have a pull-request at the end of this week at least. Thanks, Inki Dae > > Gustavo > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hi Inki, 2015-06-17 Inki Dae : > Hi Gustavo, > > On 2015ë 06ì 17ì¼ 05:35, Gustavo Padovan wrote: > > HI Inki, > > > > 2015-06-15 Inki Dae : > > > >> Hi Gustavo, > >> > >> On 2015ë 06ì 02ì¼ 00:04, Gustavo Padovan wrote: > >>> From: Gustavo Padovan > >>> > >>> Hi, > >>> > >>> Here goes the full support for atomic modesetting on exynos. I've > >>> split the patches in the various phases of atomic support. > >>> > >>> v2: fixes comments by Joonyoung > >>> - remove unused var in patch 09 > >>> - use ->disable instead of outdated ->dpms in hdmi code > >>> - remove WARN_ON from crtc enable/disable > >>> > >>> v3: fixes comment by Joonyoung > >>> - move the removal of drm_helper_disable_unused_functions() to > >>> separated patch > >>> > >>> v4: add patches that remove unnecessary calls to disable_plane() > >>> > >>> v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) > >>> > >>> v6: rebase on latest exynos_drm_next > >>> > >>> v7: fix comments by Joonyoung > >>> - fix two checkpatch errors > >>> - remove extra crtc->commit() call > >>> - check for null fb on exynos_check_plane() > >>> > >>> v8: fix comments by Joonyoung > >>> - fix merge error > >>> - move drm_crtc_vblank_get to the commit that introduces atomic > >>> pageflip > >>> - remove .prepare() in the apropiated patch > >>> - add a new patch to move exynos_drm_crtc_disable() > >>> > >>> v9: * fix comments by Joonyoung > >>> - also remove encoder .prepare() > >>> - do not shift exynos_update_plane() parameters > >>> - remove unused .mode_set() and .mode_set_base() > >>> * add specific exynos .atomic_commit() > >>> * add split of exynos_crtc->ops->dpms() into enable() and disable() > >>> * add other atomic clean ups > >>> > >>> v10: * fix comments by Joonyoung > >>> - add more comments on exynos_atomic_commit() > >>> - make exynos_crtc's .enable and .disable void > >> > >> I found out one issue that refresh rate gets low with display extension > >> mode test. > >> > >> I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here > >> is how to test it, > >> > >> #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection > >> # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 > >> setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 > >> setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 > >> freq: 20.00Hz > >> freq: 20.00Hz > >> > >> As you can see, refresh rate is 20. > >> > >> Below is the result without atomic patch series, > >> # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 > >> setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 > >> setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 > >> freq: 60.18Hz > >> freq: 49.09Hz > >> > >> I can see 60Hz for FIMD and 49Hz for vidi. > > > > I gave this a try and figured out that this might be a vidi specific > > problem. If I try VIDI and FIMD I get the same results as you but with > > Mixer and FIMD(the setup I actually use to test) everything works fine. > > Hm... Did Mixer and FIMD combination really work correctly with > extension mode? Yes. collabora at singularity:~$ modetest -M exynos -s 31:1920x1080 -v -s 25:1366x768 setting mode 1920x1080-60Hz at XR24 on connectors 31, crtc 29 setting mode 1366x768-60Hz at XR24 on connectors 25, crtc 23 freq: 55.82Hz freq: 55.38Hz freq: 55.30Hz freq: 55.38Hz freq: 55.30Hz freq: 55.38Hz freq: 55.30Hz freq: 55.38Hz freq: 55.30Hz freq: 55.38Hz > > > So somehow my patches caused a regression on vidi that I'm still > > ivestigating. > > I think this issue is because page flip operations are performed in > atomic: if I see correctly, when page flip is requested by modetest, the > call flow is as follows, > > drm_atomic_helper_page_flip > drm_atomic_async_commit > exynos_atomic_commit > ... > drm_atomic_helper_wait_for_vblanks > ... > > However, the function, drm_atomic_helper_wait_for_vblanks called by > exynos_atomic_commit, waits for the vblank completion of each crtc > driver . See wait_event_timeout function call in > drm_atomic_helper_wait_for_vblanks function. > > This means that a page flip request of a crtc driver cannot be performed > until the vblank completion of another crtc driver. I think you should > have implemented exynos_atomic_commit function asynchronously, not > synchronously like you did. So it seems that this function should be > re-implemented. I have a patch for it already. I'll resend a v2 of my last series. > > If my analysis is correct and you cannot post the change set within this > week, I'm not sure whether the atomic patch series should go to > mainline. Anyway, I will decide it and have a pull-request at the end of > this week at least. We have about 3 months to fix this, until v4.2 is out, n
[PATCH v10 00/17] drm/exynos: atomic modesetting support
HI Inki, 2015-06-15 Inki Dae : > Hi Gustavo, > > On 2015ë 06ì 02ì¼ 00:04, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Hi, > > > > Here goes the full support for atomic modesetting on exynos. I've > > split the patches in the various phases of atomic support. > > > > v2: fixes comments by Joonyoung > > - remove unused var in patch 09 > > - use ->disable instead of outdated ->dpms in hdmi code > > - remove WARN_ON from crtc enable/disable > > > > v3: fixes comment by Joonyoung > > - move the removal of drm_helper_disable_unused_functions() to > > separated patch > > > > v4: add patches that remove unnecessary calls to disable_plane() > > > > v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) > > > > v6: rebase on latest exynos_drm_next > > > > v7: fix comments by Joonyoung > > - fix two checkpatch errors > > - remove extra crtc->commit() call > > - check for null fb on exynos_check_plane() > > > > v8: fix comments by Joonyoung > > - fix merge error > > - move drm_crtc_vblank_get to the commit that introduces atomic > > pageflip > > - remove .prepare() in the apropiated patch > > - add a new patch to move exynos_drm_crtc_disable() > > > > v9: * fix comments by Joonyoung > > - also remove encoder .prepare() > > - do not shift exynos_update_plane() parameters > > - remove unused .mode_set() and .mode_set_base() > > * add specific exynos .atomic_commit() > > * add split of exynos_crtc->ops->dpms() into enable() and disable() > > * add other atomic clean ups > > > > v10: * fix comments by Joonyoung > > - add more comments on exynos_atomic_commit() > > - make exynos_crtc's .enable and .disable void > > I found out one issue that refresh rate gets low with display extension > mode test. > > I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here > is how to test it, > > #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection > # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 > setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 > setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 > freq: 20.00Hz > freq: 20.00Hz > > As you can see, refresh rate is 20. > > Below is the result without atomic patch series, > # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 > setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 > setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 > freq: 60.18Hz > freq: 49.09Hz > > I can see 60Hz for FIMD and 49Hz for vidi. I gave this a try and figured out that this might be a vidi specific problem. If I try VIDI and FIMD I get the same results as you but with Mixer and FIMD(the setup I actually use to test) everything works fine. So somehow my patches caused a regression on vidi that I'm still ivestigating. Gustavo
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hi Gustavo, On 2015ë 06ì 02ì¼ 00:04, Gustavo Padovan wrote: > From: Gustavo Padovan > > Hi, > > Here goes the full support for atomic modesetting on exynos. I've > split the patches in the various phases of atomic support. > > v2: fixes comments by Joonyoung > - remove unused var in patch 09 > - use ->disable instead of outdated ->dpms in hdmi code > - remove WARN_ON from crtc enable/disable > > v3: fixes comment by Joonyoung > - move the removal of drm_helper_disable_unused_functions() to > separated patch > > v4: add patches that remove unnecessary calls to disable_plane() > > v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) > > v6: rebase on latest exynos_drm_next > > v7: fix comments by Joonyoung > - fix two checkpatch errors > - remove extra crtc->commit() call > - check for null fb on exynos_check_plane() > > v8: fix comments by Joonyoung > - fix merge error > - move drm_crtc_vblank_get to the commit that introduces atomic > pageflip > - remove .prepare() in the apropiated patch > - add a new patch to move exynos_drm_crtc_disable() > > v9: * fix comments by Joonyoung > - also remove encoder .prepare() > - do not shift exynos_update_plane() parameters > - remove unused .mode_set() and .mode_set_base() > * add specific exynos .atomic_commit() > * add split of exynos_crtc->ops->dpms() into enable() and disable() > * add other atomic clean ups > > v10: * fix comments by Joonyoung > - add more comments on exynos_atomic_commit() > - make exynos_crtc's .enable and .disable void I found out one issue that refresh rate gets low with display extension mode test. I tested it with two crtc drivers - vidi and fimd on Trats2 board. Here is how to test it, #echo 1 > /sys/devices/platform/exynos-drm-vidi/connection # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 freq: 20.00Hz freq: 20.00Hz As you can see, refresh rate is 20. Below is the result without atomic patch series, # modetest -M exynos -v -s 31 at 29:720x1280 -s 23 at 21:640x480 setting mode 720x1280-60Hz at XR24 on connectors 31, crtc 29 setting mode 640x480-75Hz at XR24 on connectors 23, crtc 21 freq: 60.18Hz freq: 49.09Hz I can see 60Hz for FIMD and 49Hz for vidi. Thanks, Inki Dae > > > Gustavo > > --- > Gustavo Padovan (15): > drm/exynos: atomic phase 1: use drm_plane_helper_update() > drm/exynos: atomic phase 1: use drm_plane_helper_disable() > drm/exynos: atomic phase 1: add .mode_set_nofb() callback > drm/exynos: atomic phase 2: wire up state reset(), duplicate() and > destroy() > drm/exynos: atomic phase 2: keep track of framebuffer pointer > drm/exynos: atomic phase 3: atomic updates of planes > drm/exynos: atomic phase 3: use atomic .set_config helper > drm/exynos: atomic phase 3: convert page flips > drm/exynos: remove exported functions from exynos_drm_plane > drm/exynos: don't disable unused functions at init > drm/exynos: move exynos_drm_crtc_disable() > drm/exynos: add exynos specific .atomic_commit() > drm/exynos: atomic dpms support > drm/exynos: remove unnecessary calls to disable_plane() > drm/exynos: split exynos_crtc->dpms in enable() and disable() > > Joonyoung Shim (2): > drm/exynos: fix source data argument for plane > drm/exynos: use adjusted_mode of crtc_state instead of mode > > drivers/gpu/drm/bridge/ps8622.c | 6 +- > drivers/gpu/drm/bridge/ptn3460.c| 6 +- > drivers/gpu/drm/exynos/exynos7_drm_decon.c | 91 +++-- > drivers/gpu/drm/exynos/exynos_dp_core.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_crtc.c| 201 > ++-- > drivers/gpu/drm/exynos/exynos_drm_dpi.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 + > drivers/gpu/drm/exynos/exynos_drm_drv.h | 10 +- > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +- > drivers/gpu/drm/exynos/exynos_drm_encoder.c | 35 + > drivers/gpu/drm/exynos/exynos_drm_fb.c | 41 ++ > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 3 - > drivers/gpu/drm/exynos/exynos_drm_fimd.c| 71 +++--- > drivers/gpu/drm/exynos/exynos_drm_plane.c | 137 ++- > drivers/gpu/drm/exynos/exynos_drm_plane.h | 11 -- > drivers/gpu/drm/exynos/exynos_drm_vidi.c| 59 > drivers/gpu/drm/exynos/exynos_hdmi.c| 10 +- > drivers/gpu/drm/exynos/exynos_mixer.c | 26 +--- > 18 files changed, 269 insertions(+), 458 deletions(-) >
[PATCH v10 00/17] drm/exynos: atomic modesetting support
On 06/11/2015 11:01 PM, Gustavo Padovan wrote:
> Hi Joonyoung,
>
> 2015-06-11 Joonyoung Shim :
>
>> On 06/10/2015 10:36 PM, Gustavo Padovan wrote:
>>> Hi Marek,
>>>
>>> 2015-06-10 Marek Szyprowski :
>>>
Hello,
On 2015-06-01 17:04, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> 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 0xbc50 plane->src_width 640 plane->src_height 480
^C
[ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
[ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
After applying atomic modesetting patchset:
# modetest -M exynos -s 24:640x480
[ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
[ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
[ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
[ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
^C
[ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
As you can see from the above log, mixer_graph_buffer function is called
several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
and 0xbc50 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.
[PATCH v10 00/17] drm/exynos: atomic modesetting support
On 06/10/2015 10:36 PM, Gustavo Padovan wrote:
> Hi Marek,
>
> 2015-06-10 Marek Szyprowski :
>
>> Hello,
>>
>> On 2015-06-01 17:04, Gustavo Padovan wrote:
>>> From: Gustavo Padovan
>>>
>>> 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 0xbc50 plane->src_width 640 plane->src_height 480
>> ^C
>> [ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>> [ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>>
>> After applying atomic modesetting patchset:
>> # modetest -M exynos -s 24:640x480
>> [ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>> [ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>> setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
>> [ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
>> [ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
>> ^C
>> [ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>>
>> As you can see from the above log, mixer_graph_buffer function is called
>> several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
>> and 0xbc50 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.
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hi Joonyoung,
2015-06-11 Joonyoung Shim :
> On 06/10/2015 10:36 PM, Gustavo Padovan wrote:
> > Hi Marek,
> >
> > 2015-06-10 Marek Szyprowski :
> >
> >> Hello,
> >>
> >> On 2015-06-01 17:04, Gustavo Padovan wrote:
> >>> From: Gustavo Padovan
> >>>
> >>> 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 0xbc50 plane->src_width 640 plane->src_height 480
> >> ^C
> >> [ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> >> [ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> >>
> >> After applying atomic modesetting patchset:
> >> # modetest -M exynos -s 24:640x480
> >> [ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> >> [ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> >> setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
> >> [ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
> >> [ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
> >> ^C
> >> [ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> >>
> >> As you can see from the above log, mixer_graph_buffer function is called
> >> several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
> >> and 0xbc50 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.
Gustavo
[PATCH v10 00/17] drm/exynos: atomic modesetting support
On 2015ë
06ì 10ì¼ 20:38, Marek Szyprowski wrote:
> 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
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 0xbc50 plane->src_width 640 plane->src_height 480
>>> ^C
>>> [ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height
>>> 1080
>>> [ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height
>>> 1080
>>>
>>> After applying atomic modesetting patchset:
>>> # modetest -M exynos -s 24:640x480
>>> [ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height
>>> 1080
>>> [ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height
>>> 1080
>>> setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
>>> [ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height
>>> 1080
>>> [ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
>>> ^C
>>> [ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height
>>> 1080
>>>
>>> As you can see from the above log, mixer_graph_buffer function is called
>>> several times. 0xbbd0 is the DMA address of the 1920x1080
>>> framebuffer
>>> and 0xbc50 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 at 11c0 {
>> ...
>> iommus = <&sysmmu_fimd0>;
>> ...
>>
>> sysmmu_fimd0: sysmmu at 11E2 {
>> compatible = "samsung,exynos-sysmmu";
>> reg = <0x11E2 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 at 11c0 {
>> status = "okay";
>> iommu-reserved-mapping = <0x4000 0x4000
>> 0x4000>;
>> };
>>
>> 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
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hi Marek,
On 2015ë
06ì 10ì¼ 19:03, Marek Szyprowski wrote:
> Hello,
>
> On 2015-06-01 17:04, Gustavo Padovan wrote:
>> From: Gustavo Padovan
>>
>> 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 0xbc50 plane->src_width 640 plane->src_height 480
> ^C
> [ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> [ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>
> After applying atomic modesetting patchset:
> # modetest -M exynos -s 24:640x480
> [ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> [ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
> [ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
> [ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
> ^C
> [ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>
> As you can see from the above log, mixer_graph_buffer function is called
> several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
> and 0xbc50 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 at 11c0 {
...
iommus = <&sysmmu_fimd0>;
...
sysmmu_fimd0: sysmmu at 11E2 {
compatible = "samsung,exynos-sysmmu";
reg = <0x11E2 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 at 11c0 {
status = "okay";
iommu-reserved-mapping = <0x4000 0x4000 0x4000>;
};
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.
Thanks,
Inki Dae
>
>> (...)
>
> Best regards
[PATCH v10 00/17] drm/exynos: atomic modesetting support
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
>>>
>>> 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 0xbc50 plane->src_width 640 plane->src_height 480
>> ^C
>> [ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>> [ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>>
>> After applying atomic modesetting patchset:
>> # modetest -M exynos -s 24:640x480
>> [ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>> [ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>> setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
>> [ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
>> [ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
>> ^C
>> [ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>>
>> As you can see from the above log, mixer_graph_buffer function is called
>> several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
>> and 0xbc50 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 at 11c0 {
> ...
> iommus = <&sysmmu_fimd0>;
> ...
>
> sysmmu_fimd0: sysmmu at 11E2 {
> compatible = "samsung,exynos-sysmmu";
> reg = <0x11E2 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 at 11c0 {
> status = "okay";
> iommu-reserved-mapping = <0x4000 0x4000 0x4000>;
> };
>
> 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-4
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hello,
On 2015-06-01 17:04, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> 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 0xbc50 plane->src_width 640 plane->src_height 480
^C
[ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
[ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
After applying atomic modesetting patchset:
# modetest -M exynos -s 24:640x480
[ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
[ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
[ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
[ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
^C
[ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
As you can see from the above log, mixer_graph_buffer function is called
several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
and 0xbc50 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?
> (...)
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
[PATCH v10 00/17] drm/exynos: atomic modesetting support
Hi Marek,
2015-06-10 Marek Szyprowski :
> Hello,
>
> On 2015-06-01 17:04, Gustavo Padovan wrote:
> >From: Gustavo Padovan
> >
> >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 0xbc50 plane->src_width 640 plane->src_height 480
> ^C
> [ 3870.555232] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> [ 3870.565696] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>
> After applying atomic modesetting patchset:
> # modetest -M exynos -s 24:640x480
> [ 142.540122] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> [ 142.550726] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
> setting mode 640x480-60Hz at XR24 on connectors 24, crtc 22
> [ 142.643672] dma 0xbc50 plane->src_width 1920 plane->src_height 1080
> [ 142.759982] dma 0xbc50 plane->src_width 640 plane->src_height 480
> ^C
> [ 154.986040] dma 0xbbd0 plane->src_width 1920 plane->src_height 1080
>
> As you can see from the above log, mixer_graph_buffer function is called
> several times. 0xbbd0 is the DMA address of the 1920x1080 framebuffer
> and 0xbc50 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.
Gustavo
[PATCH v10 00/17] drm/exynos: atomic modesetting support
From: Gustavo Padovan Hi, Here goes the full support for atomic modesetting on exynos. I've split the patches in the various phases of atomic support. v2: fixes comments by Joonyoung - remove unused var in patch 09 - use ->disable instead of outdated ->dpms in hdmi code - remove WARN_ON from crtc enable/disable v3: fixes comment by Joonyoung - move the removal of drm_helper_disable_unused_functions() to separated patch v4: add patches that remove unnecessary calls to disable_plane() v5: fixes NULL CRTC crash on planes updates (reported by Inki and Tobias) v6: rebase on latest exynos_drm_next v7: fix comments by Joonyoung - fix two checkpatch errors - remove extra crtc->commit() call - check for null fb on exynos_check_plane() v8: fix comments by Joonyoung - fix merge error - move drm_crtc_vblank_get to the commit that introduces atomic pageflip - remove .prepare() in the apropiated patch - add a new patch to move exynos_drm_crtc_disable() v9: * fix comments by Joonyoung - also remove encoder .prepare() - do not shift exynos_update_plane() parameters - remove unused .mode_set() and .mode_set_base() * add specific exynos .atomic_commit() * add split of exynos_crtc->ops->dpms() into enable() and disable() * add other atomic clean ups v10: * fix comments by Joonyoung - add more comments on exynos_atomic_commit() - make exynos_crtc's .enable and .disable void Gustavo --- Gustavo Padovan (15): drm/exynos: atomic phase 1: use drm_plane_helper_update() drm/exynos: atomic phase 1: use drm_plane_helper_disable() drm/exynos: atomic phase 1: add .mode_set_nofb() callback drm/exynos: atomic phase 2: wire up state reset(), duplicate() and destroy() drm/exynos: atomic phase 2: keep track of framebuffer pointer drm/exynos: atomic phase 3: atomic updates of planes drm/exynos: atomic phase 3: use atomic .set_config helper drm/exynos: atomic phase 3: convert page flips drm/exynos: remove exported functions from exynos_drm_plane drm/exynos: don't disable unused functions at init drm/exynos: move exynos_drm_crtc_disable() drm/exynos: add exynos specific .atomic_commit() drm/exynos: atomic dpms support drm/exynos: remove unnecessary calls to disable_plane() drm/exynos: split exynos_crtc->dpms in enable() and disable() Joonyoung Shim (2): drm/exynos: fix source data argument for plane drm/exynos: use adjusted_mode of crtc_state instead of mode drivers/gpu/drm/bridge/ps8622.c | 6 +- drivers/gpu/drm/bridge/ptn3460.c| 6 +- drivers/gpu/drm/exynos/exynos7_drm_decon.c | 91 +++-- drivers/gpu/drm/exynos/exynos_dp_core.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_crtc.c| 201 ++-- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 + drivers/gpu/drm/exynos/exynos_drm_drv.h | 10 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 35 + drivers/gpu/drm/exynos/exynos_drm_fb.c | 41 ++ drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 3 - drivers/gpu/drm/exynos/exynos_drm_fimd.c| 71 +++--- drivers/gpu/drm/exynos/exynos_drm_plane.c | 137 ++- drivers/gpu/drm/exynos/exynos_drm_plane.h | 11 -- drivers/gpu/drm/exynos/exynos_drm_vidi.c| 59 drivers/gpu/drm/exynos/exynos_hdmi.c| 10 +- drivers/gpu/drm/exynos/exynos_mixer.c | 26 +--- 18 files changed, 269 insertions(+), 458 deletions(-) -- 2.1.0
