drm: rockchip: CONFIG_DRM_FBDEV_EMULATION will crash the HDMI down sometimes
Hello: I want to enable the video output at RK3288 Firefly board, but I found if I enable CONFIG_DRM_FBDEV_EMULATION, the HDMI would crash down sometimes but sometimes it works. After disable that opinion, I never meet a problem. I have not verified it with eDP as I meet a big problem in there. [ OK ] Stopped LSB: Load kernel modules needed to enable cpufreq scaling. [ 33.282843] [drm:drm_atomic_helper_commit_cleanup_done [drm_kms_helper]] *ERROR* [CRTC:27:crtc-0] flip_done timed out [ 33.361489] [ cut here ] [ 33.366251] WARNING: CPU: 2 PID: 476 at /home/ayaka/workspace/rk3288/kernel/linux-kernel/drivers/gpu/drm/rockchip/rockchip_drm$ vop.c:730 vop_plane_atomic_update+0x1218/0x177c [rockchipdrm] [ 33.383071] Modules linked in: rockchip_vop_reg rockchipdrm dw_hdmi_rockchip dw_hdmi drm_kms_helper cfbfillrect mali_kbase sys$ opyarea gpio_ir_recv cfbimgblt sysfillrect rc_core dwc2 sysimgblt fb_sys_fops panel_simple cfbcopyarea rk_crypto drm nvmem_rockch$ p_efuse udc_core des_generic phy_rockchip_usb pwm_rockchip pwm_bl backlight fb [ 33.412751] CPU: 2 PID: 476 Comm: X Tainted: GW 4.10.0-rc3-next-20170111+ #148 [ 33.421182] Hardware name: Rockchip (Device Tree) [ 33.425905] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 33.433652] [] (show_stack) from [] (dump_stack+0x8c/0xa0) [ 33.440881] [] (dump_stack) from [] (__warn+0xf8/0x110) [ 33.447839] [] (__warn) from [] (warn_slowpath_null+0x30/0x38) [ 33.455431] [] (warn_slowpath_null) from [] (vop_plane_atomic_update+0x1218/0x177c [rockchipdrm]) [ 33.466126] [] (vop_plane_atomic_update [rockchipdrm]) from [] (drm_atomic_helper_commit_planes+0xd4/0x2b8 [drm_kms_helper]) [ 33.479160] [] (drm_atomic_helper_commit_planes [drm_kms_helper]) from [] (rockchip_atomic_commit_tail+0x4$ /0x68 [rockchipdrm]) [ 33.492451] [] (rockchip_atomic_commit_tail [rockchipdrm]) from [] (commit_tail+0x50/0xb8 [drm_kms_helper]$ [ 33.504061] [] (commit_tail [drm_kms_helper]) from [] (drm_atomic_helper_commit+0xd4/0x13c [drm_kms_helper$ ) [ 33.515850] [] (drm_atomic_helper_commit [drm_kms_helper]) from [] (drm_atomic_commit+0x5c/0x68 [drm]) [ 33.527126] [] (drm_atomic_commit [drm]) from [] (restore_fbdev_mode+0x160/0x300 [drm_kms_helper]) [ 33.537952] [] (restore_fbdev_mode [drm_kms_helper]) from [] (drm_fb_helper_restore_fbdev_mode_unlocked+0x$ 0/0x84 [drm_kms_helper]) [ 33.551590] [] (drm_fb_helper_restore_fbdev_mode_unlocked [drm_kms_helper]) from [] (rockchip_drm_lastclos$ +0x1c/0x20 [rockchipdrm]) [ 33.565404] [] (rockchip_drm_lastclose [rockchipdrm]) from [] (drm_lastclose+0x48/0xd8 [drm]) [ 33.575977] [] (drm_lastclose [drm]) from [] (drm_release+0x2c4/0x36c [drm]) [ 33.584927] [] (drm_release [drm]) from [] (__fput+0x9c/0x1e8) [ 33.592501] [] (__fput) from [] (fput+0x18/0x1c) [ 33.599206] [] (fput) from [] (task_work_run+0xcc/0xf0) [ 33.606519] [] (task_work_run) from [] (do_work_pending+0xd0/0xd4) [ 33.614439] [] (do_work_pending) from [] (slow_work_pending+0xc/0x20) [ 33.622674] ---[ end trace 95ed2c3f167607d2 ]--- [ 33.627327] [ cut here ] [ 33.632013] WARNING: CPU: 2 PID: 476 at /home/ayaka/workspace/rk3288/kernel/linux-kernel/drivers/gpu/drm/rockchip/rockchip_drm_ vop.c:1017 vop_crtc_atomic_flush+0x27c/0x2b8 [rockchipdrm] [ 33.648542] Modules linked in: rockchip_vop_reg rockchipdrm dw_hdmi_rockchip dw_hdmi drm_kms_helper cfbfillrect mali_kbase sysc opyarea gpio_ir_recv cfbimgblt sysfillrect rc_core dwc2 sysimgblt fb_sys_fops panel_simple cfbcopyarea rk_crypto drm nvmem_rockchi p_efuse udc_core des_generic phy_rockchip_usb pwm_rockchip pwm_bl backlight fb [ 33.678191] CPU: 2 PID: 476 Comm: X Tainted: GW 4.10.0-rc3-next-20170111+ #148 [ 33.686621] Hardware name: Rockchip (Device Tree) [ 33.691339] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 33.699084] [] (show_stack) from [] (dump_stack+0x8c/0xa0) [ 33.706309] [] (dump_stack) from [] (__warn+0xf8/0x110) [ 33.713275] [] (__warn) from [] (warn_slowpath_null+0x30/0x38) [ 33.720864] [] (warn_slowpath_null) from [] (vop_crtc_atomic_flush+0x27c/0x2b8 [rockchipdrm]) [ 33.731208] [] (vop_crtc_atomic_flush [rockchipdrm]) from [] (drm_atomic_helper_commit_planes+0x1d0/0x2b8 [ drm_kms_helper]) [ 33.744159] [] (drm_atomic_helper_commit_planes [drm_kms_helper]) from [] (rockchip_atomic_commit_tail+0x44 /0x68 [rockchipdrm]) [ 33.757451] [] (rockchip_atomic_commit_tail [rockchipdrm]) from [] (commit_tail+0x50/0xb8 [drm_kms_helper]) [ 33.769064] [] (commit_tail [drm_kms_helper]) from [] (drm_atomic_helper_commit+0xd4/0x13c [drm_kms_helper] ) [ 33.780861] [] (drm_atomic_helper_commit [drm_kms_helper]) from [] (drm_atomic_commit+0x5c/0x68 [drm]) [ 33.792131] [] (drm_atomic_commit [drm]) from [] (restore_fbdev_mode+0x160/0x300 [drm_kms_helper]) [ 33.802956] [] (restore_fbdev_mode [drm_kms_helper]) from
Re: [patch] drm/vc4: fix a bounds check
On Mon, Jan 16, 2017 at 11:40:10PM +, Eric Engestrom wrote: > > diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c > > b/drivers/gpu/drm/vc4/vc4_render_cl.c > > index 08886a3..5cdd003 100644 > > --- a/drivers/gpu/drm/vc4/vc4_render_cl.c > > +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c > > @@ -461,7 +461,7 @@ static int vc4_rcl_surface_setup(struct vc4_exec_info > > *exec, > > } > > > > ret = vc4_full_res_bounds_check(exec, *obj, surf); > > - if (!ret) > > + if (ret) > > return ret; > > > > return 0; > > This now boils down to `return vc4_full_res_bounds_check(...);`, so you > could get rid of the `ret` variable completely :) I tried to leave it the original style which the author intended. That's also my prefered style. I like big chunky "return 0;". I actually found this bug by looking at places where people return a variable where a literal would work: if (!ret) return ret; It's ambiguos if they intended to return a negative, or if they condition is reversed. The other reason why I slightly prefer his style is because people get so "clever" with the last condition in a function and it drives me nuts. They'll do a series of checks like this: if (fail) goto; if (fail) goto; if (fail) goto; if (success) return; label: We should be testing for failure generally, but this particular kind of success check is like nails on a chalk board for me. My younger self is guilty of this cleverness as well Of course, the other way: "return vc4_full_res_bounds_check();" is fine too. It's not something that bothers me. I guess I just would do whatever the original author prefers. regards, dan carpenter ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/7] drm/atomic: Add new iterators over all state, v3.
Op 17-01-17 om 00:11 schreef Laurent Pinchart: > Hi Maarten, > > Thank you for the patch. > > On Monday 16 Jan 2017 10:37:38 Maarten Lankhorst wrote: >> Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to >> replace the old for_each_xxx_in_state ones. This is useful for >1 flip >> depth and getting rid of all xxx->state dereferences. >> >> This requires extra fixups done when committing a state after >> duplicating, which in general isn't valid but is used by suspend/resume. >> To handle these, introduce drm_atomic_helper_commit_duplicated_state >> which performs those fixups before checking & committing the state. >> >> Changes since v1: >> - Remove nonblock parameter for commit_duplicated_state. >> Changes since v2: >> - Use commit_duplicated_state for i915 load detection. >> - Add WARN_ON(old_state != obj->state) before swapping. >> >> Signed-off-by: Maarten Lankhorst>> --- >> drivers/gpu/drm/drm_atomic.c | 6 +++ >> drivers/gpu/drm/drm_atomic_helper.c | 65 + >> drivers/gpu/drm/i915/intel_display.c | 13 +++--- >> include/drm/drm_atomic.h | 81 +++-- >> include/drm/drm_atomic_helper.h | 2 + >> 5 files changed, 149 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c >> index 6414bcf7f41b..1c1cbf436717 100644 >> --- a/drivers/gpu/drm/drm_atomic.c >> +++ b/drivers/gpu/drm/drm_atomic.c >> @@ -275,6 +275,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state >> *state, return ERR_PTR(-ENOMEM); >> >> state->crtcs[index].state = crtc_state; >> +state->crtcs[index].old_state = crtc->state; >> +state->crtcs[index].new_state = crtc_state; >> state->crtcs[index].ptr = crtc; >> crtc_state->state = state; >> >> @@ -691,6 +693,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state >> *state, >> >> state->planes[index].state = plane_state; >> state->planes[index].ptr = plane; >> +state->planes[index].old_state = plane->state; >> +state->planes[index].new_state = plane_state; >> plane_state->state = state; >> >> DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n", >> @@ -1031,6 +1035,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state >> *state, >> >> drm_connector_reference(connector); >> state->connectors[index].state = connector_state; >> +state->connectors[index].old_state = connector->state; >> +state->connectors[index].new_state = connector_state; >> state->connectors[index].ptr = connector; >> connector_state->state = state; >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> b/drivers/gpu/drm/drm_atomic_helper.c index b26e3419027e..d153e8a72921 >> 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -1971,11 +1971,11 @@ void drm_atomic_helper_swap_state(struct >> drm_atomic_state *state, int i; >> long ret; >> struct drm_connector *connector; >> -struct drm_connector_state *conn_state; >> +struct drm_connector_state *conn_state, *old_conn_state; >> struct drm_crtc *crtc; >> -struct drm_crtc_state *crtc_state; >> +struct drm_crtc_state *crtc_state, *old_crtc_state; >> struct drm_plane *plane; >> -struct drm_plane_state *plane_state; >> +struct drm_plane_state *plane_state, *old_plane_state; >> struct drm_crtc_commit *commit; >> >> if (stall) { >> @@ -1999,13 +1999,17 @@ void drm_atomic_helper_swap_state(struct >> drm_atomic_state *state, } >> } >> >> -for_each_connector_in_state(state, connector, conn_state, i) { >> +for_each_oldnew_connector_in_state(state, connector, old_conn_state, >> conn_state, i) { >> +WARN_ON(connector->state != old_conn_state); >> + >> connector->state->state = state; >> swap(state->connectors[i].state, connector->state); >> connector->state->state = NULL; >> } >> >> -for_each_crtc_in_state(state, crtc, crtc_state, i) { >> +for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, >> i) { >> +WARN_ON(crtc->state != old_crtc_state); >> + >> crtc->state->state = state; >> swap(state->crtcs[i].state, crtc->state); >> crtc->state->state = NULL; >> @@ -2020,7 +2024,9 @@ void drm_atomic_helper_swap_state(struct >> drm_atomic_state *state, } >> } >> >> -for_each_plane_in_state(state, plane, plane_state, i) { >> +for_each_oldnew_plane_in_state(state, plane, old_plane_state, >> plane_state, i) { >> +WARN_ON(plane->state != old_plane_state); >> + >> plane->state->state = state; >> swap(state->planes[i].state, plane->state); >> plane->state->state = NULL; >> @@ -2471,7 +2477,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all); >> * >> * See also: >> * drm_atomic_helper_duplicate_state(),
[Bug 99418] DRI3 Stuttering while scrolling in Chromium/Chrome with VBLANK off
https://bugs.freedesktop.org/show_bug.cgi?id=99418 --- Comment #1 from Michel Dänzer--- Please attach the corresponding Xorg log file and output of glxinfo. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: use atomic helper commit
2017년 01월 17일 05:34에 Laurent Pinchart 이(가) 쓴 글: > Hi Inki, > > Thank you for the patch. > > On Monday 16 Jan 2017 18:13:22 Inki Dae wrote: >> This patch relpaces specific atomic commit function >> with atomic helper commit one, which also includes >> atomic_commit_tail callback for Exynos SoC becasue >> crtc devices on Exynos SoC uses power domain device >> so drm_atomic_helper_commit_planes should be called >> after drm_atomic_helper_commit_modeset_enables. > > Please note that drm_atomic_helper_commit() is currently broken, its async > commit support is subject to a race condition. Maarten's "[PATCH v3 0/7] > drm/atomic: Add accessor macros for all atomic state" patch series is an > attempt to fix that, I'll try to review it ASAP. Thanks for share and if you give me a review, it would help me a lot. > >> Signed-off-by: Inki Dae>> --- >> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 9 ++- >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 110 +--- >> drivers/gpu/drm/exynos/exynos_drm_fb.c | 25 ++- >> 3 files changed, 33 insertions(+), 111 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 2530bf5..47da612 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> @@ -39,6 +39,14 @@ static void exynos_drm_crtc_disable(struct drm_crtc >> *crtc) >> >> if (exynos_crtc->ops->disable) >> exynos_crtc->ops->disable(exynos_crtc); >> + >> +if (crtc->state->event && !crtc->state->active) { >> +spin_lock_irq(>dev->event_lock); >> +drm_crtc_send_vblank_event(crtc, crtc->state->event); >> +spin_unlock_irq(>dev->event_lock); >> + >> +crtc->state->event = NULL; >> +} >> } >> >> static void >> @@ -94,7 +102,6 @@ static void exynos_crtc_atomic_flush(struct drm_crtc >> *crtc, drm_crtc_send_vblank_event(crtc, event); >> spin_unlock_irqrestore(>dev->event_lock, flags); >> } >> - >> } >> >> static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = { >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 3ec0535..9d0df00 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> @@ -38,56 +38,6 @@ >> #define DRIVER_MAJOR1 >> #define DRIVER_MINOR0 >> >> -struct exynos_atomic_commit { >> -struct work_struct work; >> -struct drm_device *dev; >> -struct drm_atomic_state *state; >> -u32 crtcs; >> -}; >> - >> -static void exynos_atomic_commit_complete(struct exynos_atomic_commit >> *commit) -{ >> -struct drm_device *dev = commit->dev; >> -struct exynos_drm_private *priv = dev->dev_private; >> -struct drm_atomic_state *state = commit->state; >> - >> -drm_atomic_helper_commit_modeset_disables(dev, state); >> - >> -drm_atomic_helper_commit_modeset_enables(dev, state); >> - >> -/* >> - * Exynos can't update planes with CRTCs and encoders disabled, >> - * its updates routines, specially for FIMD, requires the clocks >> - * to be enabled. So it is necessary to handle the modeset operations >> - * *before* the commit_planes() step, this way it will always >> - * have the relevant clocks enabled to perform the update. >> - */ >> - >> -drm_atomic_helper_commit_planes(dev, state, 0); >> - >> -drm_atomic_helper_wait_for_vblanks(dev, state); >> - >> -drm_atomic_helper_cleanup_planes(dev, state); >> - >> -drm_atomic_state_put(state); >> - >> -spin_lock(>lock); >> -priv->pending &= ~commit->crtcs; >> -spin_unlock(>lock); >> - >> -wake_up_all(>wait); >> - >> -kfree(commit); >> -} >> - >> -static void exynos_drm_atomic_work(struct work_struct *work) >> -{ >> -struct exynos_atomic_commit *commit = container_of(work, >> -struct exynos_atomic_commit, work); >> - >> -exynos_atomic_commit_complete(commit); >> -} >> - >> static struct device *exynos_drm_get_dma_device(void); >> >> static int exynos_drm_load(struct drm_device *dev, unsigned long flags) >> @@ -202,65 +152,6 @@ static void exynos_drm_unload(struct drm_device *dev) >> dev->dev_private = NULL; >> } >> >> -static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs) >> -{ >> -bool pending; >> - >> -spin_lock(>lock); >> -pending = priv->pending & crtcs; >> -spin_unlock(>lock); >> - >> -return pending; >> -} >> - >> -int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state >> *state, - bool nonblock) >> -{ >> -struct exynos_drm_private *priv = dev->dev_private; >> -struct exynos_atomic_commit *commit; >> -struct drm_crtc *crtc; >> -struct drm_crtc_state *crtc_state; >> -int i, ret; >> - >> -commit = kzalloc(sizeof(*commit),
Re: [PATCH] drm/exynos: use atomic helper commit
2017년 01월 16일 20:22에 Tobias Jakobi 이(가) 쓴 글: > Inki Dae wrote: >> This patch relpaces specific atomic commit function >> with atomic helper commit one, which also includes >> atomic_commit_tail callback for Exynos SoC becasue >> crtc devices on Exynos SoC uses power domain device >> so drm_atomic_helper_commit_planes should be called >> after drm_atomic_helper_commit_modeset_enables. > The commit message needs fixing. I think I know my way around Exynos DRM > a bit, but reading this just confuses me. Seems I didn't describe it. Thanks for pointing. Will fix it. Thanks. > > In particular the first part can probably be dropped, since it only > describes what the patch does (and I can already see this from the diff > itself). > Also some spelling issues: > relpaces -> replaces > becasue - because > > With best wishes, > Tobias > > >> >> Signed-off-by: Inki Dae>> --- >> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 9 ++- >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 110 >> +-- >> drivers/gpu/drm/exynos/exynos_drm_fb.c | 25 ++- >> 3 files changed, 33 insertions(+), 111 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> index 2530bf5..47da612 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c >> @@ -39,6 +39,14 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc) >> >> if (exynos_crtc->ops->disable) >> exynos_crtc->ops->disable(exynos_crtc); >> + >> +if (crtc->state->event && !crtc->state->active) { >> +spin_lock_irq(>dev->event_lock); >> +drm_crtc_send_vblank_event(crtc, crtc->state->event); >> +spin_unlock_irq(>dev->event_lock); >> + >> +crtc->state->event = NULL; >> +} >> } >> >> static void >> @@ -94,7 +102,6 @@ static void exynos_crtc_atomic_flush(struct drm_crtc >> *crtc, >> drm_crtc_send_vblank_event(crtc, event); >> spin_unlock_irqrestore(>dev->event_lock, flags); >> } >> - >> } >> >> static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = { >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> index 3ec0535..9d0df00 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> @@ -38,56 +38,6 @@ >> #define DRIVER_MAJOR1 >> #define DRIVER_MINOR0 >> >> -struct exynos_atomic_commit { >> -struct work_struct work; >> -struct drm_device *dev; >> -struct drm_atomic_state *state; >> -u32 crtcs; >> -}; >> - >> -static void exynos_atomic_commit_complete(struct exynos_atomic_commit >> *commit) >> -{ >> -struct drm_device *dev = commit->dev; >> -struct exynos_drm_private *priv = dev->dev_private; >> -struct drm_atomic_state *state = commit->state; >> - >> -drm_atomic_helper_commit_modeset_disables(dev, state); >> - >> -drm_atomic_helper_commit_modeset_enables(dev, state); >> - >> -/* >> - * Exynos can't update planes with CRTCs and encoders disabled, >> - * its updates routines, specially for FIMD, requires the clocks >> - * to be enabled. So it is necessary to handle the modeset operations >> - * *before* the commit_planes() step, this way it will always >> - * have the relevant clocks enabled to perform the update. >> - */ >> - >> -drm_atomic_helper_commit_planes(dev, state, 0); >> - >> -drm_atomic_helper_wait_for_vblanks(dev, state); >> - >> -drm_atomic_helper_cleanup_planes(dev, state); >> - >> -drm_atomic_state_put(state); >> - >> -spin_lock(>lock); >> -priv->pending &= ~commit->crtcs; >> -spin_unlock(>lock); >> - >> -wake_up_all(>wait); >> - >> -kfree(commit); >> -} >> - >> -static void exynos_drm_atomic_work(struct work_struct *work) >> -{ >> -struct exynos_atomic_commit *commit = container_of(work, >> -struct exynos_atomic_commit, work); >> - >> -exynos_atomic_commit_complete(commit); >> -} >> - >> static struct device *exynos_drm_get_dma_device(void); >> >> static int exynos_drm_load(struct drm_device *dev, unsigned long flags) >> @@ -202,65 +152,6 @@ static void exynos_drm_unload(struct drm_device *dev) >> dev->dev_private = NULL; >> } >> >> -static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs) >> -{ >> -bool pending; >> - >> -spin_lock(>lock); >> -pending = priv->pending & crtcs; >> -spin_unlock(>lock); >> - >> -return pending; >> -} >> - >> -int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state >> *state, >> - bool nonblock) >> -{ >> -struct exynos_drm_private *priv = dev->dev_private; >> -struct exynos_atomic_commit *commit; >> -struct drm_crtc *crtc; >> -struct drm_crtc_state
[Bug 93760] radeonsi vaapi mpeg2 decode slightly corrupt or asserts.
https://bugs.freedesktop.org/show_bug.cgi?id=93760 Nayan Deshmukhchanged: What|Removed |Added Assignee|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org QA Contact|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: remove unnecessary codes
2017년 01월 16일 18:37에 Andrzej Hajda 이(가) 쓴 글: > On 16.01.2017 10:13, Inki Dae wrote: >> This patch removes exynos_drm_crtc_cancel_page_flip call >> when drm is closed because at that time, events will be released >> by drm_events_release function. >> >> Change-Id: I156ea27a4c90aa87a27a50415515fa334148c912 >> Signed-off-by: Inki Dae>> --- >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 -- >> 1 file changed, 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> index 9d0df00..035d02e 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> @@ -198,13 +198,7 @@ static int exynos_drm_open(struct drm_device *dev, >> struct drm_file *file) >> static void exynos_drm_preclose(struct drm_device *dev, >> struct drm_file *file) >> { >> -struct drm_crtc *crtc; >> - >> exynos_drm_subdrv_close(dev, file); >> - >> -list_for_each_entry(crtc, >mode_config.crtc_list, head) >> -exynos_drm_crtc_cancel_page_flip(crtc, file); > > Since this is single user of exynos_drm_crtc_cancel_page_flip, this > function can be removed as well. Indeed. Will remove that function. Thanks. > Beside this: > Reviewed-by: Andrzej Hajda > > > Regards > Andrzej > >> - >> } >> >> static void exynos_drm_postclose(struct drm_device *dev, struct drm_file >> *file) > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/atomic: Save flip flags in drm_plane_state
Allows using atomic flip helpers for drivers using ASYNC flip. Remove ASYNC_FLIP restriction in helpers and caches the page flip flags in drm_plane_state to be used in the low level drivers. v2: Resending the patch since the original was broken. Remove comment about not supporting ASYNC flips. Change-Id: I0219c3ec3ecceb82143ee176d30cb50d9aa76bf0 Signed-off-by: Andrey Grodzovsky--- drivers/gpu/drm/drm_atomic_helper.c | 18 +- include/drm/drm_plane.h | 8 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..12f70f2 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2737,7 +2737,8 @@ static int page_flip_common( struct drm_atomic_state *state, struct drm_crtc *crtc, struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event) + struct drm_pending_vblank_event *event, + uint32_t flags) { struct drm_plane *plane = crtc->primary; struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static int page_flip_common( if (IS_ERR(plane_state)) return PTR_ERR(plane_state); + plane_state->pflip_flags = flags; ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); if (ret != 0) @@ -2781,10 +2783,6 @@ static int page_flip_common( * Provides a default _crtc_funcs.page_flip implementation * using the atomic driver interface. * - * Note that for now so called async page flips (i.e. updates which are not - * synchronized to vblank) are not supported, since the atomic interfaces have - * no provisions for this yet. - * * Returns: * Returns 0 on success, negative errno numbers on failure. * @@ -2800,9 +2798,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, struct drm_atomic_state *state; int ret = 0; - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - return -EINVAL; - state = drm_atomic_state_alloc(plane->dev); if (!state) return -ENOMEM; @@ -2810,7 +2805,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); retry: - ret = page_flip_common(state, crtc, fb, event); + ret = page_flip_common(state, crtc, fb, event, flags); if (ret != 0) goto fail; @@ -2865,9 +2860,6 @@ int drm_atomic_helper_page_flip_target( struct drm_crtc_state *crtc_state; int ret = 0; - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - return -EINVAL; - state = drm_atomic_state_alloc(plane->dev); if (!state) return -ENOMEM; @@ -2875,7 +2867,7 @@ int drm_atomic_helper_page_flip_target( state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); retry: - ret = page_flip_common(state, crtc, fb, event); + ret = page_flip_common(state, crtc, fb, event, flags); if (ret != 0) goto fail; diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index db3bbde..86d8ffc 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -122,6 +122,14 @@ struct drm_plane_state { */ bool visible; + + /** +* @pflip_flags: +* +* Flip related config options +*/ + u32 pflip_flags; + struct drm_atomic_state *state; }; -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 06/12] drm/mediatek: update display module connections
Hi, YT: On Wed, 2017-01-11 at 14:51 +0800, YT Shen wrote: > update connections for OVL, RDMA, BLS, DSI > > Signed-off-by: YT ShenAcked-by: CK Hu > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > index b77d456..a9b209c 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c > @@ -32,6 +32,10 @@ > #define DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN 0x0c8 > #define DISP_REG_CONFIG_MMSYS_CG_CON00x100 > > +#define DISP_REG_CONFIG_DISP_OVL_MOUT_EN 0x030 > +#define DISP_REG_CONFIG_OUT_SEL 0x04c > +#define DISP_REG_CONFIG_DSI_SEL 0x050 > + > #define DISP_REG_MUTEX_EN(n) (0x20 + 0x20 * (n)) > #define DISP_REG_MUTEX(n)(0x24 + 0x20 * (n)) > #define DISP_REG_MUTEX_RST(n)(0x28 + 0x20 * (n)) > @@ -71,6 +75,10 @@ > #define DPI0_SEL_IN_RDMA10x1 > #define COLOR1_SEL_IN_OVL1 0x1 > > +#define OVL_MOUT_EN_RDMA 0x1 > +#define BLS_TO_DSI_RDMA1_TO_DPI1 0x8 > +#define DSI_SEL_IN_BLS 0x0 > + > struct mtk_disp_mutex { > int id; > bool claimed; > @@ -111,6 +119,9 @@ static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id > cur, > if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) { > *addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN; > value = OVL0_MOUT_EN_COLOR0; > + } else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) { > + *addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN; > + value = OVL_MOUT_EN_RDMA; > } else if (cur == DDP_COMPONENT_OD && next == DDP_COMPONENT_RDMA0) { > *addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN; > value = OD_MOUT_EN_RDMA0; > @@ -148,6 +159,9 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id > cur, > } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) { > *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN; > value = COLOR1_SEL_IN_OVL1; > + } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) { > + *addr = DISP_REG_CONFIG_DSI_SEL; > + value = DSI_SEL_IN_BLS; > } else { > value = 0; > } > @@ -155,6 +169,15 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id > cur, > return value; > } > > +static void mtk_ddp_sout_sel(void __iomem *config_regs, > + enum mtk_ddp_comp_id cur, > + enum mtk_ddp_comp_id next) > +{ > + if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) > + writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1, > +config_regs + DISP_REG_CONFIG_OUT_SEL); > +} > + > void mtk_ddp_add_comp_to_path(void __iomem *config_regs, > enum mtk_ddp_comp_id cur, > enum mtk_ddp_comp_id next) > @@ -167,6 +190,8 @@ void mtk_ddp_add_comp_to_path(void __iomem *config_regs, > writel_relaxed(reg, config_regs + addr); > } > > + mtk_ddp_sout_sel(config_regs, cur, next); > + > value = mtk_ddp_sel_in(cur, next, ); > if (value) { > reg = readl_relaxed(config_regs + addr) | value; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/amd/display: Switch to using atomic_helper for flip.
Signed-off-by: Andrey Grodzovsky--- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 ++ 2 files changed, 6 insertions(+), 87 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 4c0a86e..3ff3c14 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -443,7 +443,6 @@ struct amdgpu_crtc { enum amdgpu_interrupt_state vsync_timer_enabled; int otg_inst; - uint32_t flip_flags; /* After Set Mode target will be non-NULL */ struct dc_target *target; }; diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index a443b70..d4664bf 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( return 0; } - -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, - struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, - uint32_t flags) -{ - struct drm_plane *plane = crtc->primary; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); - struct drm_atomic_state *state; - struct drm_plane_state *plane_state; - struct drm_crtc_state *crtc_state; - int ret = 0; - - state = drm_atomic_state_alloc(plane->dev); - if (!state) - return -ENOMEM; - - ret = drm_crtc_vblank_get(crtc); - if (ret) - return ret; - - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); -retry: - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); - goto fail; - } - crtc_state->event = event; - - plane_state = drm_atomic_get_plane_state(state, plane); - if (IS_ERR(plane_state)) { - ret = PTR_ERR(plane_state); - goto fail; - } - - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); - if (ret != 0) - goto fail; - drm_atomic_set_fb_for_plane(plane_state, fb); - - /* Make sure we don't accidentally do a full modeset. */ - state->allow_modeset = false; - if (!crtc_state->active) { - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", -crtc->base.id); - ret = -EINVAL; - goto fail; - } - acrtc->flip_flags = flags; - - ret = drm_atomic_nonblocking_commit(state); - -fail: - if (ret == -EDEADLK) - goto backoff; - - if (ret) - drm_crtc_vblank_put(crtc); - - drm_atomic_state_put(state); - - return ret; -backoff: - drm_atomic_state_clear(state); - drm_atomic_legacy_backoff(state); - - /* -* Someone might have exchanged the framebuffer while we dropped locks -* in the backoff code. We need to fix up the fb refcount tracking the -* core does for us. -*/ - plane->old_fb = plane->fb; - - goto retry; -} - /* Implemented only the options currently availible for the driver */ static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { .reset = drm_atomic_helper_crtc_reset, @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, .destroy = amdgpu_dm_crtc_destroy, .gamma_set = amdgpu_dm_atomic_crtc_gamma_set, .set_config = drm_atomic_helper_set_config, - .page_flip = amdgpu_atomic_helper_page_flip, + .page_flip_target = drm_atomic_helper_page_flip_target, .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, .atomic_set_property = dm_crtc_funcs_atomic_set_property @@ -1679,7 +1602,7 @@ static bool page_flip_needed( sizeof(old_state_tmp)) == 0 ? true:false; if (new_state->crtc && page_flip_required == false) { acrtc_new = to_amdgpu_crtc(new_state->crtc); - if (acrtc_new->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) + if (new_state->pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) page_flip_required = true; } return page_flip_required; @@ -2760,7 +2683,6 @@ int amdgpu_dm_atomic_commit( for_each_plane_in_state(state, plane, old_plane_state, i) { struct drm_plane_state *plane_state = plane->state; struct drm_crtc *crtc = plane_state->crtc; - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); struct drm_framebuffer
[PATCH v2 3/3] drm/nouveau/kms/nv50: Switch to using atomic helper for flip.
Change-Id: I5a3189c03e389af2ff6c13d870a7d28282b7b0ee Signed-off-by: Andrey Grodzovsky--- drivers/gpu/drm/nouveau/nv50_display.c | 77 +++--- 1 file changed, 5 insertions(+), 72 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 2c2c645..419e00c 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -846,6 +846,10 @@ struct nv50_wndw_func { asyw->image.w = fb->base.width; asyw->image.h = fb->base.height; asyw->image.kind = (fb->nvbo->tile_flags & 0xff00) >> 8; + + asyw->interval = + asyw->state.pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC ? 0 : 1; + if (asyw->image.kind) { asyw->image.layout = 0; if (drm->device.info.chipset >= 0xc0) @@ -2221,77 +2225,6 @@ struct nv50_base { .atomic_check = nv50_head_atomic_check, }; -/* This is identical to the version in the atomic helpers, except that - * it supports non-vblanked ("async") page flips. - */ -static int -nv50_head_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, - struct drm_pending_vblank_event *event, u32 flags) -{ - struct drm_plane *plane = crtc->primary; - struct drm_atomic_state *state; - struct drm_plane_state *plane_state; - struct drm_crtc_state *crtc_state; - int ret = 0; - - state = drm_atomic_state_alloc(plane->dev); - if (!state) - return -ENOMEM; - - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); -retry: - crtc_state = drm_atomic_get_crtc_state(state, crtc); - if (IS_ERR(crtc_state)) { - ret = PTR_ERR(crtc_state); - goto fail; - } - crtc_state->event = event; - - plane_state = drm_atomic_get_plane_state(state, plane); - if (IS_ERR(plane_state)) { - ret = PTR_ERR(plane_state); - goto fail; - } - - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); - if (ret != 0) - goto fail; - drm_atomic_set_fb_for_plane(plane_state, fb); - - /* Make sure we don't accidentally do a full modeset. */ - state->allow_modeset = false; - if (!crtc_state->active) { - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", -crtc->base.id); - ret = -EINVAL; - goto fail; - } - - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) - nv50_wndw_atom(plane_state)->interval = 0; - - ret = drm_atomic_nonblocking_commit(state); -fail: - if (ret == -EDEADLK) - goto backoff; - - drm_atomic_state_put(state); - return ret; - -backoff: - drm_atomic_state_clear(state); - drm_atomic_legacy_backoff(state); - - /* -* Someone might have exchanged the framebuffer while we dropped locks -* in the backoff code. We need to fix up the fb refcount tracking the -* core does for us. -*/ - plane->old_fb = plane->fb; - - goto retry; -} - static int nv50_head_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t size) @@ -2386,7 +2319,7 @@ struct nv50_base { .gamma_set = nv50_head_gamma_set, .destroy = nv50_head_destroy, .set_config = drm_atomic_helper_set_config, - .page_flip = nv50_head_page_flip, + .page_flip = drm_atomic_helper_page_flip, .set_property = drm_atomic_helper_crtc_set_property, .atomic_duplicate_state = nv50_head_atomic_duplicate_state, .atomic_destroy_state = nv50_head_atomic_destroy_state, -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] Allow ASYNC flip with atomic helpers.
This series is a folow-up on https://patchwork.kernel.org/patch/9501787/ The first patch makes changes to atomic helpers to allow for drives with ASYNC flip support to use them. Patch 2 is to use this in AMDGPU/DC. Patch 3 is possible cleanup in nouveau/kms who seems to have to duplicate the helper as we did to support ASYNC flips. v2: Resend drm/atomic: Save flip flags in drm_plane_state since the original patch was incomplete. Squash 2 AMD changes into one to not break compilation. Andrey Grodzovsky (3): drm/atomic: Save flip flags in drm_plane_state drm/amd/display: Switch to using atomic_helper for flip. drm/nouveau/kms/nv50: Switch to using atomic helper for flip. drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 ++ drivers/gpu/drm/drm_atomic_helper.c| 18 ++--- drivers/gpu/drm/nouveau/nv50_display.c | 77 ++ include/drm/drm_plane.h| 8 ++ 5 files changed, 24 insertions(+), 172 deletions(-) -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 05/12] drm/mediatek: add BLS component
Hi, YT: On Wed, 2017-01-11 at 14:51 +0800, YT Shen wrote: > Add BLS component for PWM + GAMMA function > > Signed-off-by: YT ShenAcked-by: CK Hu > --- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 5 - > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 2 ++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index 3ff788c..f6e853a 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -255,6 +255,7 @@ static void mtk_gamma_set(struct mtk_ddp_comp *comp, > [MTK_DISP_PWM] = "pwm", > [MTK_DISP_MUTEX] = "mutex", > [MTK_DISP_OD] = "od", > + [MTK_DISP_BLS] = "bls", > }; > > struct mtk_ddp_comp_match { > @@ -265,6 +266,7 @@ struct mtk_ddp_comp_match { > > static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] > = { > [DDP_COMPONENT_AAL] = { MTK_DISP_AAL, 0, _aal }, > + [DDP_COMPONENT_BLS] = { MTK_DISP_BLS, 0, NULL }, > [DDP_COMPONENT_COLOR0] = { MTK_DISP_COLOR, 0, _color }, > [DDP_COMPONENT_COLOR1] = { MTK_DISP_COLOR, 1, _color }, > [DDP_COMPONENT_DPI0]= { MTK_DPI,0, NULL }, > @@ -336,7 +338,8 @@ int mtk_ddp_comp_init(struct device *dev, struct > device_node *node, > comp->id = comp_id; > comp->funcs = funcs ?: mtk_ddp_matches[comp_id].funcs; > > - if (comp_id == DDP_COMPONENT_DPI0 || > + if (comp_id == DDP_COMPONENT_BLS || > + comp_id == DDP_COMPONENT_DPI0 || > comp_id == DDP_COMPONENT_DSI0 || > comp_id == DDP_COMPONENT_PWM0) { > comp->regs = NULL; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > index 22a33ee..0828cf8 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > @@ -36,11 +36,13 @@ enum mtk_ddp_comp_type { > MTK_DISP_PWM, > MTK_DISP_MUTEX, > MTK_DISP_OD, > + MTK_DISP_BLS, > MTK_DDP_COMP_TYPE_MAX, > }; > > enum mtk_ddp_comp_id { > DDP_COMPONENT_AAL, > + DDP_COMPONENT_BLS, > DDP_COMPONENT_COLOR0, > DDP_COMPONENT_COLOR1, > DDP_COMPONENT_DPI0, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/8] drm/msm/mdp5: Configure COLOR3_OUT propagation
In MDP5 Layer Mixer HW, the blender output is only the blended color components (i.e R, G and B, or COLOR0/1/2 in MDP5 HW terminology). This is fed to the BG input of the next blender. We also need to provide an alpha (COLOR3) value for the BG input at the next stage. This is configured via using the REG_MDP5_LM_BLEND_COLOR_OUT register. For each stage, we can propagate either the BG or FG alpha to the next stage. The approach taken by the driver is to propagate FG alpha, if the plane staged on that blender has an alpha. If it doesn't, we try to propagate the base layer's alpha. This is borrowed from downstream MDP5 kernel driver. Without this, we don't see any cursor plane content. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index 7a47209b0c6f..fec966293f9a 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -177,6 +177,21 @@ static void mdp5_crtc_destroy(struct drm_crtc *crtc) kfree(mdp5_crtc); } +static inline u32 mdp5_lm_use_fg_alpha_mask(enum mdp_mixer_stage_id stage) +{ + switch (stage) { + case STAGE0: return MDP5_LM_BLEND_COLOR_OUT_STAGE0_FG_ALPHA; + case STAGE1: return MDP5_LM_BLEND_COLOR_OUT_STAGE1_FG_ALPHA; + case STAGE2: return MDP5_LM_BLEND_COLOR_OUT_STAGE2_FG_ALPHA; + case STAGE3: return MDP5_LM_BLEND_COLOR_OUT_STAGE3_FG_ALPHA; + case STAGE4: return MDP5_LM_BLEND_COLOR_OUT_STAGE4_FG_ALPHA; + case STAGE5: return MDP5_LM_BLEND_COLOR_OUT_STAGE5_FG_ALPHA; + case STAGE6: return MDP5_LM_BLEND_COLOR_OUT_STAGE6_FG_ALPHA; + default: + return 0; + } +} + /* * blend_setup() - blend all the planes of a CRTC * @@ -197,6 +212,8 @@ static void blend_setup(struct drm_crtc *crtc) unsigned long flags; enum mdp5_pipe stage[STAGE_MAX + 1] = { SSPP_NONE }; int i, plane_cnt = 0; + bool bg_alpha_enabled = false; + u32 mixer_op_mode = 0; #define blender(stage) ((stage) - STAGE0) hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg); @@ -218,6 +235,11 @@ static void blend_setup(struct drm_crtc *crtc) if (!pstates[STAGE_BASE]) { ctl_blend_flags |= MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT; DBG("Border Color is enabled"); + } else if (plane_cnt) { + format = to_mdp_format(msm_framebuffer_format(pstates[STAGE_BASE]->base.fb)); + + if (format->alpha_enable) + bg_alpha_enabled = true; } /* The reset for blending */ @@ -232,6 +254,12 @@ static void blend_setup(struct drm_crtc *crtc) MDP5_LM_BLEND_OP_MODE_BG_ALPHA(BG_CONST); fg_alpha = pstates[i]->alpha; bg_alpha = 0xFF - pstates[i]->alpha; + + if (!format->alpha_enable && bg_alpha_enabled) + mixer_op_mode = 0; + else + mixer_op_mode |= mdp5_lm_use_fg_alpha_mask(i); + DBG("Stage %d fg_alpha %x bg_alpha %x", i, fg_alpha, bg_alpha); if (format->alpha_enable && pstates[i]->premultiplied) { @@ -268,6 +296,8 @@ static void blend_setup(struct drm_crtc *crtc) blender(i)), bg_alpha); } + mdp5_write(mdp5_kms, REG_MDP5_LM_BLEND_COLOR_OUT(lm), mixer_op_mode); + mdp5_ctl_blend(mdp5_crtc->ctl, stage, plane_cnt, ctl_blend_flags); out: -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 7/8] drm/msm/mdp5: Refactor mdp5_plane_atomic_check
In mdp5_plane_atomic_check, we get crtc_state from drm_plane_state. Later, for cursor planes, we'll populate the update_plane() func that takes a fast asynchronous path to implement cursor movements. There, we would need to call a similar atomic_check func to validate the plane state, but crtc_state would need to be derived differently. Refactor mdp5_plane_atomic_check to mdp5_plane_atomic_check_with_state such that the latter takes crtc_state as an argument. This is similar to what the intel driver has done for async cursor updates. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 32 +++ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index 7409e959d810..504201b710d9 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -270,17 +270,16 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane, } #define FRAC_16_16(mult, div)(((mult) << 16) / (div)) -static int mdp5_plane_atomic_check(struct drm_plane *plane, - struct drm_plane_state *state) +static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state, + struct drm_plane_state *state) { struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state); + struct drm_plane *plane = state->plane; struct drm_plane_state *old_state = plane->state; struct mdp5_cfg *config = mdp5_cfg_get_config(get_kms(plane)->cfg); bool new_hwpipe = false; uint32_t max_width, max_height; uint32_t caps = 0; - struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; struct drm_rect clip; int min_scale, max_scale; int ret; @@ -288,10 +287,6 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane, DBG("%s: check (%d -> %d)", plane->name, plane_enabled(old_state), plane_enabled(state)); - crtc = state->crtc ? state->crtc : plane->state->crtc; - if (!crtc) - return 0; - max_width = config->hw->lm.max_width << 16; max_height = config->hw->lm.max_height << 16; @@ -303,10 +298,6 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane, return -ERANGE; } - crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); - if (WARN_ON(!crtc_state)) - return -EINVAL; - clip.x1 = 0; clip.y1 = 0; clip.x2 = crtc_state->adjusted_mode.hdisplay; @@ -382,6 +373,23 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane, return 0; } +static int mdp5_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + + crtc = state->crtc ? state->crtc : plane->state->crtc; + if (!crtc) + return 0; + + crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); + if (WARN_ON(!crtc_state)) + return -EINVAL; + + return mdp5_plane_atomic_check_with_state(crtc_state, state); +} + static void mdp5_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 8/8] HACK: drm/msm/mdp5: Add support for legacy cursor updates
This code has been more or less picked up from the vc4 and intel implementations of update_plane() funcs for cursor planes. The update_plane() func is usually the drm_atomic_helper_update_plane func that will issue an atomic commit with the plane updates. Such commits are not intended to be done faster than the vsync rate. The legacy cursor userspace API, on the other hand, expects the kernel to handle cursor updates immediately. Create a fast path in update_plane, which updates the cursor registers and flushes the configuration. The fast path is taken when there is only a change in the cursor's position in the crtc, or a change in the cursor's crop co-ordinates. For anything else, we go via the slow path. We take the slow path even when the fb changes, and when there is currently no fb tied to the plane. This should hopefully ensure that we always take a slow path for every new fb. This in turn should ensure that the fb is pinned/prepared. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 7 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 1 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 110 +- 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index 84fcb6e3a176..d0c8b38b96ce 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -778,6 +778,13 @@ void mdp5_crtc_set_pipeline(struct drm_crtc *crtc, mdp5_ctl_set_pipeline(ctl, intf, lm); } +struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc) +{ + struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc); + + return mdp5_crtc->ctl; +} + int mdp5_crtc_get_lm(struct drm_crtc *crtc) { struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc); diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h index 0396e8adb693..9de471191eba 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h @@ -246,6 +246,7 @@ enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane); struct drm_plane *mdp5_plane_init(struct drm_device *dev, enum drm_plane_type type); +struct mdp5_ctl *mdp5_crtc_get_ctl(struct drm_crtc *crtc); uint32_t mdp5_crtc_vblank(struct drm_crtc *crtc); int mdp5_crtc_get_lm(struct drm_crtc *crtc); diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index 504201b710d9..0ffb8affef35 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -31,6 +31,14 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_rect *src, struct drm_rect *dest); +static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane, + struct drm_crtc *crtc, + struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h); + static void set_scanout_locked(struct drm_plane *plane, struct drm_framebuffer *fb); @@ -243,6 +251,19 @@ static const struct drm_plane_funcs mdp5_plane_funcs = { .atomic_print_state = mdp5_plane_atomic_print_state, }; +static const struct drm_plane_funcs mdp5_cursor_plane_funcs = { + .update_plane = mdp5_update_cursor_plane_legacy, + .disable_plane = drm_atomic_helper_disable_plane, + .destroy = mdp5_plane_destroy, + .set_property = drm_atomic_helper_plane_set_property, + .atomic_set_property = mdp5_plane_atomic_set_property, + .atomic_get_property = mdp5_plane_atomic_get_property, + .reset = mdp5_plane_reset, + .atomic_duplicate_state = mdp5_plane_duplicate_state, + .atomic_destroy_state = mdp5_plane_destroy_state, + .atomic_print_state = mdp5_plane_atomic_print_state, +}; + static int mdp5_plane_prepare_fb(struct drm_plane *plane, struct drm_plane_state *new_state) { @@ -860,6 +881,82 @@ static int mdp5_plane_mode_set(struct drm_plane *plane, return ret; } +static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane, + struct drm_crtc *crtc, struct drm_framebuffer *fb, + int crtc_x, int crtc_y, + unsigned int crtc_w, unsigned int crtc_h, + uint32_t src_x, uint32_t src_y, + uint32_t src_w, uint32_t src_h) +{ + struct drm_plane_state *plane_state, *new_plane_state; + struct mdp5_plane_state *mdp5_pstate; + struct drm_crtc_state *crtc_state = crtc->state; + int
[PATCH v2 6/8] drm/msm/mdp5: Add cursor planes
Register cursor drm_planes. The loop in modeset_init that inits the planes and crtcs has to be refactored a bit. We first iterate all the hwpipes to find the cursor planes. Then, we loop again to create crtcs. In msm_atomic_wait_for_commit_done, remove the check which bypasses waiting for vsyncs if state->legacy_cursor_updates is true. We will later create a fast path for cursor position changes in the cursor plane's update_plane func that doesn't go via the regular atomic commit path. For rest of cursor related updates, we will have to wait for vsyncs, so ignore the legacy_cursor_updates flag. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 33 ++--- drivers/gpu/drm/msm/msm_atomic.c| 5 - 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 70a1950de8ae..3eb0749223d9 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -411,7 +411,9 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) struct msm_drm_private *priv = dev->dev_private; const struct mdp5_cfg_hw *hw_cfg; unsigned int num_crtcs; - int i, ret; + int i, ret, pi = 0, ci = 0; + struct drm_plane *primary[MAX_BASES] = { NULL }; + struct drm_plane *cursor[MAX_BASES] = { NULL }; hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg); @@ -438,13 +440,14 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) * planes for the CRTCs, with the remainder as overlay planes: */ for (i = 0; i < mdp5_kms->num_hwpipes; i++) { - bool primary = i < num_crtcs; + struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i]; struct drm_plane *plane; - struct drm_crtc *crtc; enum drm_plane_type type; - if (primary) + if (i < num_crtcs) type = DRM_PLANE_TYPE_PRIMARY; + else if (hwpipe->caps & MDP_PIPE_CAP_CURSOR) + type = DRM_PLANE_TYPE_CURSOR; else type = DRM_PLANE_TYPE_OVERLAY; @@ -456,10 +459,16 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) } priv->planes[priv->num_planes++] = plane; - if (!primary) - continue; + if (type == DRM_PLANE_TYPE_PRIMARY) + primary[pi++] = plane; + if (type == DRM_PLANE_TYPE_CURSOR) + cursor[ci++] = plane; + } + + for (i = 0; i < num_crtcs; i++) { + struct drm_crtc *crtc; - crtc = mdp5_crtc_init(dev, plane, NULL, i); + crtc = mdp5_crtc_init(dev, primary[i], cursor[i], i); if (IS_ERR(crtc)) { ret = PTR_ERR(crtc); dev_err(dev->dev, "failed to construct crtc %d (%d)\n", i, ret); @@ -791,6 +800,9 @@ static int hwpipe_init(struct mdp5_kms *mdp5_kms) static const enum mdp5_pipe dma_planes[] = { SSPP_DMA0, SSPP_DMA1, }; + static const enum mdp5_pipe cursor_planes[] = { + SSPP_CURSOR0, SSPP_CURSOR1, + }; const struct mdp5_cfg_hw *hw_cfg; int ret; @@ -814,6 +826,13 @@ static int hwpipe_init(struct mdp5_kms *mdp5_kms) if (ret) return ret; + /* Construct cursor pipes: */ + ret = construct_pipes(mdp5_kms, hw_cfg->pipe_cursor.count, + cursor_planes, hw_cfg->pipe_cursor.base, + hw_cfg->pipe_cursor.caps); + if (ret) + return ret; + return 0; } diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 6924fa28f04f..9633a68b14d7 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -93,11 +93,6 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev, if (!crtc->state->enable) continue; - /* Legacy cursor ioctls are completely unsynced, and userspace -* relies on that (by doing tons of cursor updates). */ - if (old_state->legacy_cursor_update) - continue; - kms->funcs->wait_for_crtc_commit_done(kms, crtc); } } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/8] drm/msm/mdp5: cfg: Add pipe_cursor block
Define the block in advance so that the generated mdp5.xml.h doesn't break build. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h index bd4fe81d6160..b1c7daaede86 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h @@ -85,6 +85,7 @@ struct mdp5_cfg_hw { struct mdp5_pipe_block pipe_vig; struct mdp5_pipe_block pipe_rgb; struct mdp5_pipe_block pipe_dma; + struct mdp5_pipe_block pipe_cursor; struct mdp5_lm_block lm; struct mdp5_sub_block dspp; struct mdp5_sub_block ad; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 5/8] drm/msm/mdp5: Misc cursor plane bits
These are various changes added in preparation for cursor planes: - Add a pipe_cursor block for 8x96 in mdp5_cfg. - Add a new pipe CAP called MDP_PIPE_CAP_CURSOR. Use this to ensure we assign a cursor SSPP for a drm_plane with type DRM_PLANE_TYPE_CURSOR. - Update mdp5_ctl_blend_mask/ext_blend_mask funcs to incorporate cursor SSPPs. - In mdp5_ctl_blend, iterate through MAX_STAGES instead of stage_cnt, we need to do this because we can now have empty stages in between. - In mdp5_crtc_atomic_check, make sure that the cursor plane has the highest zorder, and stage the cursor plane to the maximum stage # present on the HW. - Create drm_crtc_funcs that doesn't try to implement cursors using the older LM cursor HW. - Pass drm_plane_type in mdp5_plane_init instead of a bool telling whether plane is primary or not. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 10 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 34 +++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c | 10 +++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 10 +++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 7 +-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c | 8 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 8 +--- drivers/gpu/drm/msm/mdp/mdp_kms.h | 1 + 8 files changed, 75 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c index 618b2ffed9b4..34ab553f6897 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c @@ -421,6 +421,16 @@ const struct mdp5_cfg_hw msm8x96_config = { MDP_PIPE_CAP_SW_PIX_EXT | 0, }, + .pipe_cursor = { + .count = 2, + .base = { 0x34000, 0x36000 }, + .caps = MDP_PIPE_CAP_HFLIP | + MDP_PIPE_CAP_VFLIP | + MDP_PIPE_CAP_SW_PIX_EXT | + MDP_PIPE_CAP_CURSOR | + 0, + }, + .lm = { .count = 6, .base = { 0x44000, 0x45000, 0x46000, 0x47000, 0x48000, 0x49000 }, diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index fec966293f9a..84fcb6e3a176 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -400,6 +400,7 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc, struct plane_state pstates[STAGE_MAX + 1]; const struct mdp5_cfg_hw *hw_cfg; const struct drm_plane_state *pstate; + bool cursor_plane = false; int cnt = 0, base = 0, i; DBG("%s: check", crtc->name); @@ -409,6 +410,9 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc, pstates[cnt].state = to_mdp5_plane_state(pstate); cnt++; + + if (plane->type == DRM_PLANE_TYPE_CURSOR) + cursor_plane = true; } /* assign a stage based on sorted zpos property */ @@ -420,6 +424,10 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc, if ((cnt > 0) && !is_fullscreen(state, [0].state->base)) base++; + /* trigger a warning if cursor isn't the highest zorder */ + WARN_ON(cursor_plane && + (pstates[cnt - 1].plane->type != DRM_PLANE_TYPE_CURSOR)); + /* verify that there are not too many planes attached to crtc * and that we don't have conflicting mixer stages: */ @@ -431,7 +439,10 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc, } for (i = 0; i < cnt; i++) { - pstates[i].state->stage = STAGE_BASE + i + base; + if (cursor_plane && (i == (cnt - 1))) + pstates[i].state->stage = hw_cfg->lm.nb_stages; + else + pstates[i].state->stage = STAGE_BASE + i + base; DBG("%s: assign pipe %s on stage=%d", crtc->name, pstates[i].plane->name, pstates[i].state->stage); @@ -642,6 +653,16 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = { .cursor_move = mdp5_crtc_cursor_move, }; +static const struct drm_crtc_funcs mdp5_crtc_no_lm_cursor_funcs = { + .set_config = drm_atomic_helper_set_config, + .destroy = mdp5_crtc_destroy, + .page_flip = drm_atomic_helper_page_flip, + .set_property = drm_atomic_helper_crtc_set_property, + .reset = drm_atomic_helper_crtc_reset, + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, +}; + static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = { .mode_set_nofb = mdp5_crtc_mode_set_nofb, .disable = mdp5_crtc_disable,
[PATCH v2 0/8] drm/msm/mdp5: Cursor plane stuff
This series does some mdp5_plane related clean ups (use plane helpers for clipping etc), adds MDP5 bits needed for cursor plane blocks, and then add cursor planes. On older MDP5 versions, we had cursor HW in Layer Mixer blocks, and that's implemented in mdp5_crtc.c. With newer hardware, the cursor blocks look exactly like MDP5 pipes (SSPPs). The "faster than vblank cursor position update stuff" has been copied from vc4 and the patches posted recently by Maarten for Intel: https://patchwork.kernel.org/patch/9466417/ Changes in v2: - Dropped generated headers patch. - Add the fast path patch in the end as suggested by Maarten. - Minor cleanups. Archit Taneja (8): drm/msm/mdp5: cfg: Add pipe_cursor block drm/msm/mdp5: Prepare CRTC/LM for empty stages drm/msm/mdp5: Use plane helpers to configure src/dst rectangles drm/msm/mdp5: Configure COLOR3_OUT propagation drm/msm/mdp5: Misc cursor plane bits drm/msm/mdp5: Add cursor planes drm/msm/mdp5: Refactor mdp5_plane_atomic_check HACK: drm/msm/mdp5: Add support for legacy cursor updates drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 10 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 1 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 73 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c | 14 ++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h | 4 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 39 +-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 8 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.c | 8 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 181 +++--- drivers/gpu/drm/msm/mdp/mdp_kms.h | 1 + drivers/gpu/drm/msm/msm_atomic.c | 26 +++-- 11 files changed, 316 insertions(+), 49 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/8] drm/msm/mdp5: Prepare CRTC/LM for empty stages
Use SSPP_NONE in mdp5_plane_pipe() if there is now hwpipe allocated for the drm_plane. Returning '0' means we are returning VIG0 pipe. Also, use the mdp5_pipe enum to pass around the stage array. Initialize the stage to SSPP_NONE by default. We do the above because 1) Cursor plane has to be staged at the topmost blender of the LM, which can result in empty stages in between 2) In the future, when we support multiple LMs per CRTC. We could have stages which don't have any pipe assigned to them. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 2 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c | 4 ++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h | 4 ++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c index 1ce8a01a5a28..7a47209b0c6f 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c @@ -195,7 +195,7 @@ static void blend_setup(struct drm_crtc *crtc) uint32_t lm = mdp5_crtc->lm; uint32_t blend_op, fg_alpha, bg_alpha, ctl_blend_flags = 0; unsigned long flags; - uint8_t stage[STAGE_MAX + 1]; + enum mdp5_pipe stage[STAGE_MAX + 1] = { SSPP_NONE }; int i, plane_cnt = 0; #define blender(stage) ((stage) - STAGE0) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c index d021edc3b307..ab339ce7425a 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c @@ -351,8 +351,8 @@ static u32 mdp_ctl_blend_ext_mask(enum mdp5_pipe pipe, } } -int mdp5_ctl_blend(struct mdp5_ctl *ctl, u8 *stage, u32 stage_cnt, - u32 ctl_blend_op_flags) +int mdp5_ctl_blend(struct mdp5_ctl *ctl, enum mdp5_pipe *stage, u32 stage_cnt, + u32 ctl_blend_op_flags) { unsigned long flags; u32 blend_cfg = 0, blend_ext_cfg = 0; diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h index 96148c6f863c..fda00d33e4db 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h @@ -56,8 +56,8 @@ int mdp5_ctl_pair(struct mdp5_ctl *ctlx, struct mdp5_ctl *ctly, bool enable); * (call mdp5_ctl_commit() with mdp_ctl_flush_mask_ctl() mask) */ #define MDP5_CTL_BLEND_OP_FLAG_BORDER_OUT BIT(0) -int mdp5_ctl_blend(struct mdp5_ctl *ctl, u8 *stage, u32 stage_cnt, - u32 ctl_blend_op_flags); +int mdp5_ctl_blend(struct mdp5_ctl *ctl, enum mdp5_pipe *stage, u32 stage_cnt, + u32 ctl_blend_op_flags); /** * mdp_ctl_flush_mask...() - Register FLUSH masks diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index b9fb111d3428..eb8dc7c36419 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -823,7 +823,7 @@ enum mdp5_pipe mdp5_plane_pipe(struct drm_plane *plane) struct mdp5_plane_state *pstate = to_mdp5_plane_state(plane->state); if (WARN_ON(!pstate->hwpipe)) - return 0; + return SSPP_NONE; return pstate->hwpipe->pipe; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/8] drm/msm/mdp5: Use plane helpers to configure src/dst rectangles
The MDP5 plane's atomic_check ops doesn't perform clipping tests. This didn't hurt us much in the past, but clipping becomes important with cursor planes. Use drm_plane_helper_check_state, the way rockchip/intel/mtk drivers already do. Use these drivers as reference. Clipping requires knowledge of the crtc width and height. This requires us to call drm_atomic_helper_check_modeset before drm_atomic_helper_check_planes in the driver's atomic_check op, because check_modetest will populate the mode for the crtc, needed to populate the clip rectangle. We update the plane_enabled(state) local helper to use state->visible, since state->visible and 'state->fb && state->crtc' represent the same thing. One issue with the existing code is that we don't have a way to disable the plane when it's completely clipped out. Until there isn't an update on the crtc (which would de-stage the plane), we would still see the plane in its last 'visible' configuration. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 57 --- drivers/gpu/drm/msm/msm_atomic.c | 21 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c index eb8dc7c36419..939991d5f346 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c @@ -29,10 +29,7 @@ struct mdp5_plane { static int mdp5_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h); + struct drm_rect *src, struct drm_rect *dest); static void set_scanout_locked(struct drm_plane *plane, struct drm_framebuffer *fb); @@ -45,7 +42,7 @@ static struct mdp5_kms *get_kms(struct drm_plane *plane) static bool plane_enabled(struct drm_plane_state *state) { - return state->fb && state->crtc; + return state->visible; } static void mdp5_plane_destroy(struct drm_plane *plane) @@ -272,6 +269,7 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane, msm_framebuffer_cleanup(fb, mdp5_kms->id); } +#define FRAC_16_16(mult, div)(((mult) << 16) / (div)) static int mdp5_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { @@ -281,10 +279,19 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane, bool new_hwpipe = false; uint32_t max_width, max_height; uint32_t caps = 0; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct drm_rect clip; + int min_scale, max_scale; + int ret; DBG("%s: check (%d -> %d)", plane->name, plane_enabled(old_state), plane_enabled(state)); + crtc = state->crtc ? state->crtc : plane->state->crtc; + if (!crtc) + return 0; + max_width = config->hw->lm.max_width << 16; max_height = config->hw->lm.max_height << 16; @@ -296,6 +303,22 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane, return -ERANGE; } + crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); + if (WARN_ON(!crtc_state)) + return -EINVAL; + + clip.x1 = 0; + clip.y1 = 0; + clip.x2 = crtc_state->adjusted_mode.hdisplay; + clip.y2 = crtc_state->adjusted_mode.vdisplay; + min_scale = FRAC_16_16(1, 8); + max_scale = FRAC_16_16(8, 1); + + ret = drm_plane_helper_check_state(state, , min_scale, + max_scale, true, true); + if (ret) + return ret; + if (plane_enabled(state)) { unsigned int rotation; const struct mdp_format *format; @@ -368,10 +391,7 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane, ret = mdp5_plane_mode_set(plane, state->crtc, state->fb, - state->crtc_x, state->crtc_y, - state->crtc_w, state->crtc_h, - state->src_x, state->src_y, - state->src_w, state->src_h); + >src, >dst); /* atomic_check should have ensured that this doesn't fail */ WARN_ON(ret < 0); } @@ -664,10 +684,7 @@ static void mdp5_write_pixel_ext(struct mdp5_kms *mdp5_kms, enum mdp5_pipe pipe, static int mdp5_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_framebuffer *fb, - int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x,
[PATCH] drm/exynos: g2d: fix overflow of cmdlist size
The size of cmdlist is integer type, so it can be overflowed by cmd and cmd_buf that has too big size. This patch will fix overflow issue as checking maximum size of cmd and cmd_buf. Signed-off-by: Joonyoung Shim--- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c index fbd13fa..b31244f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c @@ -1250,7 +1250,14 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data, cmdlist->data[cmdlist->last++] = G2D_INTEN_ACF; } - /* Check size of cmdlist: last 2 is about G2D_BITBLT_START */ + /* Check size of cmd and cmdlist: last 2 is about G2D_BITBLT_START */ + size = (G2D_CMDLIST_DATA_NUM - cmdlist->last - 2) / 2; + if (req->cmd_nr > size || req->cmd_buf_nr > size) { + dev_err(dev, "size of cmd or cmd_buf is too big\n"); + ret = -EINVAL; + goto err_free_event; + } + size = cmdlist->last + req->cmd_nr * 2 + req->cmd_buf_nr * 2 + 2; if (size > G2D_CMDLIST_DATA_NUM) { dev_err(dev, "cmdlist size is too big\n"); -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state
> -Original Message- > From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] > Sent: Monday, January 16, 2017 5:18 PM > To: dri-devel@lists.freedesktop.org > Cc: Grodzovsky, Andrey; nouv...@lists.freedesktop.org; amd- > g...@lists.freedesktop.org; Deucher, Alexander; daniel.vet...@intel.com > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > Hi Andrey, > > Thank you for the patch. > > On Monday 16 Jan 2017 10:44:55 Andrey Grodzovsky wrote: > > Allows using atomic flip helpers for drivers using ASYNC flip. > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > flags in drm_plane_state to be used in the low level drivers. > > > > Signed-off-by: Andrey Grodzovsky> > --- > > drivers/gpu/drm/drm_atomic_helper.c | 10 +++--- > > include/drm/drm_plane.h | 8 > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2737,7 +2737,8 @@ static int page_flip_common( > > struct drm_atomic_state *state, > > struct drm_crtc *crtc, > > struct drm_framebuffer *fb, > > - struct drm_pending_vblank_event *event) > > + struct drm_pending_vblank_event *event, > > + uint32_t flags) > > { > > struct drm_plane *plane = crtc->primary; > > struct drm_plane_state *plane_state; @@ -2754,6 +2755,7 @@ static > > int page_flip_common( > > if (IS_ERR(plane_state)) > > return PTR_ERR(plane_state); > > > > + plane_state->pflip_flags = flags; > > > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > > if (ret != 0) > > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc > > *crtc, struct drm_atomic_state *state; > > int ret = 0; > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > - return -EINVAL; > > - > > With this change all drivers using the helper will not reject that async flag, > even if they don't implement support for async page flip. You need to either > patch them all to reject the flag, or implement async page flip support for > all > of them (preferable in the helpers, and then remove the Please check drm_mode_page_flip_ioctl, one of the checks in the beginning is if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip) return -EINVAL; We in DC explicitly set dev->mode_config.async_page_flip = true, any driver which is Not supporting ASYNC flip will fail the IOCTL at this point. Same in drm_mode_atomic_ioctl > > * Note that for now so called async page flips (i.e. updates which are not > * synchronized to vblank) are not supported, since the atomic interfaces > have > * no provisions for this yet. > > comment). Thanks, that a good catch, will remove. Andrey > > > state = drm_atomic_state_alloc(plane->dev); > > if (!state) > > return -ENOMEM; > > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > > struct drm_crtc_state *crtc_state; > > int ret = 0; > > > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > > - return -EINVAL; > > - > > state = drm_atomic_state_alloc(plane->dev); > > if (!state) > > return -ENOMEM; > > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index > > db3bbde..86d8ffc 100644 > > --- a/include/drm/drm_plane.h > > +++ b/include/drm/drm_plane.h > > @@ -122,6 +122,14 @@ struct drm_plane_state { > > */ > > bool visible; > > > > + > > + /** > > +* @pflip_flags: > > +* > > +* Flip related config options > > +*/ > > + u32 pflip_flags; > > + > > struct drm_atomic_state *state; > > }; > > -- > Regards, > > Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] drm/msm/mdp5: Create single encoder per interface (INTF)
For the DSI interfaces, the mdp5_kms core creates 2 encoders for video and command modes. Create only a single encoder per interface. When creating the encoder, set the interface type to MDP5_INTF_MODE_NONE. It's the bridge (DSI/HDMI/eDP) driver's responsibility to set a different interface type. It can use the the kms func op set_encoder_mode to change the mode of operation, which in turn would configure the interface type for the INTF. In mdp5_cmd_encoder.c, we remove the redundant code, and make the commmand mode funcs as helpers that are used in mdp5_encoder.c Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c | 135 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 35 +++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 20 ++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 32 -- 4 files changed, 66 insertions(+), 156 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c index c627ab6d0061..df1c8adec3f3 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c @@ -16,16 +16,6 @@ #include "drm_crtc.h" #include "drm_crtc_helper.h" -struct mdp5_cmd_encoder { - struct drm_encoder base; - struct mdp5_interface intf; - bool enabled; - uint32_t bsc; - - struct mdp5_ctl *ctl; -}; -#define to_mdp5_cmd_encoder(x) container_of(x, struct mdp5_cmd_encoder, base) - static struct mdp5_kms *get_kms(struct drm_encoder *encoder) { struct msm_drm_private *priv = encoder->dev->dev_private; @@ -36,47 +26,8 @@ static struct mdp5_kms *get_kms(struct drm_encoder *encoder) #include #include #include -#define MDP_BUS_VECTOR_ENTRY(ab_val, ib_val) \ - { \ - .src = MSM_BUS_MASTER_MDP_PORT0,\ - .dst = MSM_BUS_SLAVE_EBI_CH0, \ - .ab = (ab_val), \ - .ib = (ib_val), \ - } - -static struct msm_bus_vectors mdp_bus_vectors[] = { - MDP_BUS_VECTOR_ENTRY(0, 0), - MDP_BUS_VECTOR_ENTRY(20, 20), -}; -static struct msm_bus_paths mdp_bus_usecases[] = { { - .num_paths = 1, - .vectors = _bus_vectors[0], -}, { - .num_paths = 1, - .vectors = _bus_vectors[1], -} }; -static struct msm_bus_scale_pdata mdp_bus_scale_table = { - .usecase = mdp_bus_usecases, - .num_usecases = ARRAY_SIZE(mdp_bus_usecases), - .name = "mdss_mdp", -}; - -static void bs_init(struct mdp5_cmd_encoder *mdp5_cmd_enc) -{ - mdp5_cmd_enc->bsc = msm_bus_scale_register_client( - _bus_scale_table); - DBG("bus scale client: %08x", mdp5_cmd_enc->bsc); -} - -static void bs_fini(struct mdp5_cmd_encoder *mdp5_cmd_enc) -{ - if (mdp5_cmd_enc->bsc) { - msm_bus_scale_unregister_client(mdp5_cmd_enc->bsc); - mdp5_cmd_enc->bsc = 0; - } -} -static void bs_set(struct mdp5_cmd_encoder *mdp5_cmd_enc, int idx) +static void bs_set(struct mdp5_encoder *mdp5_cmd_enc, int idx) { if (mdp5_cmd_enc->bsc) { DBG("set bus scaling: %d", idx); @@ -89,14 +40,12 @@ static void bs_set(struct mdp5_cmd_encoder *mdp5_cmd_enc, int idx) } } #else -static void bs_init(struct mdp5_cmd_encoder *mdp5_cmd_enc) {} -static void bs_fini(struct mdp5_cmd_encoder *mdp5_cmd_enc) {} -static void bs_set(struct mdp5_cmd_encoder *mdp5_cmd_enc, int idx) {} +static void bs_set(struct mdp5_encoder *mdp5_cmd_enc, int idx) {} #endif #define VSYNC_CLK_RATE 1920 static int pingpong_tearcheck_setup(struct drm_encoder *encoder, - struct drm_display_mode *mode) + struct drm_display_mode *mode) { struct mdp5_kms *mdp5_kms = get_kms(encoder); struct device *dev = encoder->dev->dev; @@ -176,23 +125,11 @@ static void pingpong_tearcheck_disable(struct drm_encoder *encoder) clk_disable_unprepare(mdp5_kms->vsync_clk); } -static void mdp5_cmd_encoder_destroy(struct drm_encoder *encoder) -{ - struct mdp5_cmd_encoder *mdp5_cmd_enc = to_mdp5_cmd_encoder(encoder); - bs_fini(mdp5_cmd_enc); - drm_encoder_cleanup(encoder); - kfree(mdp5_cmd_enc); -} - -static const struct drm_encoder_funcs mdp5_cmd_encoder_funcs = { - .destroy = mdp5_cmd_encoder_destroy, -}; - -static void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +void mdp5_cmd_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { - struct mdp5_cmd_encoder
[PATCH v2 5/6] drm/msm/mdp5: cfg: Change count to unsigned int
Count can't be non-zero. Changing to uint will also prevent future warnings. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h index 050e1618c836..bd4fe81d6160 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h @@ -32,7 +32,7 @@ extern const struct mdp5_cfg_hw *mdp5_cfg; typedef DECLARE_BITMAP(mdp5_smp_state_t, MAX_SMP_BLOCKS); #define MDP5_SUB_BLOCK_DEFINITION \ - int count; \ + unsigned int count; \ uint32_t base[MAX_BASES] struct mdp5_sub_block { -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 6/6] drm/msm/mdp5: Create only as many CRTCs as we need
We currently create CRTCs equaling to the # of Layer Mixer blocks we have on the MDP5 HW. This number is generally more than the # of encoders (INTFs) we have in the MDSS HW. The number of encoders connected to displays on the platform (as described by DT) would be even lesser. Create only N drm_crtcs, where N is the number of drm_encoders successfully registered. To do this, we call modeset_init_intf() before we init the drm_crtcs and drm_planes. Because of this change, setting encoder->possible_crtcs needs to be moved from construct_encoder() to a later point when we know how many CRTCs we have. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 39 - 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 9794cad131cd..ecfa38949aea 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -293,7 +293,6 @@ static struct drm_encoder *construct_encoder(struct mdp5_kms *mdp5_kms, return encoder; } - encoder->possible_crtcs = (1 << priv->num_crtcs) - 1; priv->encoders[priv->num_encoders++] = encoder; return encoder; @@ -411,16 +410,35 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) struct drm_device *dev = mdp5_kms->dev; struct msm_drm_private *priv = dev->dev_private; const struct mdp5_cfg_hw *hw_cfg; + unsigned int num_crtcs; int i, ret; hw_cfg = mdp5_cfg_get_hw_config(mdp5_kms->cfg); - /* Construct planes equaling the number of hw pipes, and CRTCs -* for the N layer-mixers (LM). The first N planes become primary + /* +* Construct encoders and modeset initialize connector devices +* for each external display interface. +*/ + for (i = 0; i < ARRAY_SIZE(hw_cfg->intf.connect); i++) { + ret = modeset_init_intf(mdp5_kms, i); + if (ret) + goto fail; + } + + /* +* We should ideally have less number of encoders (set up by parsing +* the MDP5 interfaces) than the number of layer mixers present in HW, +* but let's be safe here anyway +*/ + num_crtcs = min(priv->num_encoders, mdp5_cfg->lm.count); + + /* +* Construct planes equaling the number of hw pipes, and CRTCs for the +* N encoders set up by the driver. The first N planes become primary * planes for the CRTCs, with the remainder as overlay planes: */ for (i = 0; i < mdp5_kms->num_hwpipes; i++) { - bool primary = i < mdp5_cfg->lm.count; + bool primary = i < num_crtcs; struct drm_plane *plane; struct drm_crtc *crtc; @@ -444,13 +462,14 @@ static int modeset_init(struct mdp5_kms *mdp5_kms) priv->crtcs[priv->num_crtcs++] = crtc; } - /* Construct encoders and modeset initialize connector devices -* for each external display interface. + /* +* Now that we know the number of crtcs we've created, set the possible +* crtcs for the encoders */ - for (i = 0; i < ARRAY_SIZE(hw_cfg->intf.connect); i++) { - ret = modeset_init_intf(mdp5_kms, i); - if (ret) - goto fail; + for (i = 0; i < priv->num_encoders; i++) { + struct drm_encoder *encoder = priv->encoders[i]; + + encoder->possible_crtcs = (1 << priv->num_crtcs) - 1; } return 0; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/6] drm/msm/mdp5: Encoder related cleanups
The MDP5 and DSI drivers created 2 drm_encoders for a DSI interface (one for each mode of operation). This set creates one encoder per DSI interface. This is needed by the updated drm_bridge API. Now, with the # encoders equal to the # of displays, we can create the right # of CRTCs. We previously created LM # of CRTCs, which was a bit unnecessary. Changes in v2: - Rebase over bridge API updates going in 4.11 - Fix issues seen when calling kms func's set_encoder_mode for devices where the control/primary device is of the type mipi_dsi_device Archit Taneja (6): drm/msm: Construct only one encoder for DSI drm/msm: Set encoder's mode of operation using a kms func drm/msm/mdp5: Prepare for merging video and command encoders drm/msm/mdp5: Create single encoder per interface (INTF) drm/msm/mdp5: cfg: Change count to unsigned int drm/msm/mdp5: Create only as many CRTCs as we need drivers/gpu/drm/msm/dsi/dsi.c | 14 +-- drivers/gpu/drm/msm/dsi/dsi.h | 5 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 + drivers/gpu/drm/msm/dsi/dsi_manager.c | 61 +++ drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 +++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 2 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_cmd_encoder.c | 135 +++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 77 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 86 --- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 37 +-- drivers/gpu/drm/msm/msm_drv.h | 11 +- drivers/gpu/drm/msm/msm_kms.h | 3 + 12 files changed, 219 insertions(+), 242 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/6] drm/msm/mdp5: Prepare for merging video and command encoders
Rename the mdp5_encoder_* ops for active displays to mdp5_vid_encoder_* ops. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 31 ++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 3 ++- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 4 ++-- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c index 1e7c99125fdd..63f4135c9b5e 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c @@ -112,9 +112,9 @@ static const struct drm_encoder_funcs mdp5_encoder_funcs = { .destroy = mdp5_encoder_destroy, }; -static void mdp5_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +static void mdp5_vid_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder); struct mdp5_kms *mdp5_kms = get_kms(encoder); @@ -221,7 +221,7 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder, mdp5_encoder->ctl); } -static void mdp5_encoder_disable(struct drm_encoder *encoder) +static void mdp5_vid_encoder_disable(struct drm_encoder *encoder) { struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder); struct mdp5_kms *mdp5_kms = get_kms(encoder); @@ -256,7 +256,7 @@ static void mdp5_encoder_disable(struct drm_encoder *encoder) mdp5_encoder->enabled = false; } -static void mdp5_encoder_enable(struct drm_encoder *encoder) +static void mdp5_vid_encoder_enable(struct drm_encoder *encoder) { struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder); struct mdp5_kms *mdp5_kms = get_kms(encoder); @@ -279,6 +279,23 @@ static void mdp5_encoder_enable(struct drm_encoder *encoder) mdp5_encoder->enabled = true; } +static void mdp5_encoder_mode_set(struct drm_encoder *encoder, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + mdp5_vid_encoder_mode_set(encoder, mode, adjusted_mode); +} + +static void mdp5_encoder_disable(struct drm_encoder *encoder) +{ + mdp5_vid_encoder_disable(encoder); +} + +static void mdp5_encoder_enable(struct drm_encoder *encoder) +{ + mdp5_vid_encoder_enable(encoder); +} + static const struct drm_encoder_helper_funcs mdp5_encoder_helper_funcs = { .mode_set = mdp5_encoder_mode_set, .disable = mdp5_encoder_disable, @@ -303,8 +320,8 @@ u32 mdp5_encoder_get_framecount(struct drm_encoder *encoder) return mdp5_read(mdp5_kms, REG_MDP5_INTF_FRAME_COUNT(intf)); } -int mdp5_encoder_set_split_display(struct drm_encoder *encoder, - struct drm_encoder *slave_encoder) +int mdp5_vid_encoder_set_split_display(struct drm_encoder *encoder, + struct drm_encoder *slave_encoder) { struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder); struct mdp5_encoder *mdp5_slave_enc = to_mdp5_encoder(slave_encoder); diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index f73d46756912..b85af0f1c79c 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -148,7 +148,8 @@ static int mdp5_set_split_display(struct msm_kms *kms, return mdp5_cmd_encoder_set_split_display(encoder, slave_encoder); else - return mdp5_encoder_set_split_display(encoder, slave_encoder); + return mdp5_vid_encoder_set_split_display(encoder, + slave_encoder); } static void mdp5_set_encoder_mode(struct msm_kms *kms, diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h index 3e0baed0a8a7..f2419666c43e 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h @@ -244,8 +244,8 @@ struct drm_crtc *mdp5_crtc_init(struct drm_device *dev, struct drm_encoder *mdp5_encoder_init(struct drm_device *dev, struct mdp5_interface *intf, struct mdp5_ctl *ctl); -int mdp5_encoder_set_split_display(struct drm_encoder *encoder, - struct drm_encoder *slave_encoder); +int mdp5_vid_encoder_set_split_display(struct drm_encoder *encoder, + struct drm_encoder *slave_encoder); void mdp5_encoder_set_intf_mode(struct drm_encoder *encoder, bool cmd_mode); int mdp5_encoder_get_linecount(struct
[PATCH v2 1/6] drm/msm: Construct only one encoder for DSI
We currently create 2 encoders for DSI interfaces, one for command mode and other for video mode operation. This isn't needed as we can't really use both the encoders at the same time. It also makes connecting bridges harder. Switch to creating a single encoder. For now, we assume that the encoder is configured only in video mode. Later, the same encoder would be usable in both modes. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/dsi/dsi.c | 14 +- drivers/gpu/drm/msm/dsi/dsi.h | 4 ++-- drivers/gpu/drm/msm/dsi/dsi_manager.c | 22 -- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 28 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 20 ++-- drivers/gpu/drm/msm/msm_drv.h | 11 +++ 6 files changed, 32 insertions(+), 67 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c index 9593238ccc1b..311c1c1e7d6c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.c +++ b/drivers/gpu/drm/msm/dsi/dsi.c @@ -18,9 +18,7 @@ struct drm_encoder *msm_dsi_get_encoder(struct msm_dsi *msm_dsi) if (!msm_dsi || !msm_dsi_device_connected(msm_dsi)) return NULL; - return (msm_dsi->device_flags & MIPI_DSI_MODE_VIDEO) ? - msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID] : - msm_dsi->encoders[MSM_DSI_CMD_ENCODER_ID]; + return msm_dsi->encoder; } static int dsi_get_phy(struct msm_dsi *msm_dsi) @@ -187,14 +185,13 @@ void __exit msm_dsi_unregister(void) } int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, - struct drm_encoder *encoders[MSM_DSI_ENCODER_NUM]) +struct drm_encoder *encoder) { struct msm_drm_private *priv = dev->dev_private; struct drm_bridge *ext_bridge; - int ret, i; + int ret; - if (WARN_ON(!encoders[MSM_DSI_VIDEO_ENCODER_ID] || - !encoders[MSM_DSI_CMD_ENCODER_ID])) + if (WARN_ON(!encoder)) return -EINVAL; msm_dsi->dev = dev; @@ -205,8 +202,7 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct drm_device *dev, goto fail; } - for (i = 0; i < MSM_DSI_ENCODER_NUM; i++) - msm_dsi->encoders[i] = encoders[i]; + msm_dsi->encoder = encoder; msm_dsi->bridge = msm_dsi_manager_bridge_init(msm_dsi->id); if (IS_ERR(msm_dsi->bridge)) { diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index 03f115f532c2..ddcda8cec9a7 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -73,8 +73,8 @@ struct msm_dsi { struct device *phy_dev; bool phy_enabled; - /* the encoders we are hooked to (outside of dsi block) */ - struct drm_encoder *encoders[MSM_DSI_ENCODER_NUM]; + /* the encoder we are hooked to (outside of dsi block) */ + struct drm_encoder *encoder; int id; }; diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 2bd8dad76105..19da23d8a0b1 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -540,7 +540,7 @@ struct drm_connector *msm_dsi_manager_connector_init(u8 id) struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); struct drm_connector *connector = NULL; struct dsi_connector *dsi_connector; - int ret, i; + int ret; dsi_connector = kzalloc(sizeof(*dsi_connector), GFP_KERNEL); if (!dsi_connector) @@ -566,9 +566,7 @@ struct drm_connector *msm_dsi_manager_connector_init(u8 id) connector->interlace_allowed = 0; connector->doublescan_allowed = 0; - for (i = 0; i < MSM_DSI_ENCODER_NUM; i++) - drm_mode_connector_attach_encoder(connector, - msm_dsi->encoders[i]); + drm_mode_connector_attach_encoder(connector, msm_dsi->encoder); return connector; } @@ -591,13 +589,7 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id) dsi_bridge->id = id; - /* -* HACK: we may not know the external DSI bridge device's mode -* flags here. We'll get to know them only when the device -* attaches to the dsi host. For now, assume the bridge supports -* DSI video mode -*/ - encoder = msm_dsi->encoders[MSM_DSI_VIDEO_ENCODER_ID]; + encoder = msm_dsi->encoder; bridge = _bridge->base; bridge->funcs = _mgr_bridge_funcs; @@ -628,13 +620,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id) ext_bridge = msm_dsi->external_bridge = msm_dsi_host_get_bridge(msm_dsi->host); - /* -* HACK: we may not know the external DSI bridge device's mode -* flags here. We'll get to know them only when the device -* attaches to the dsi host. For
[PATCH v2 2/6] drm/msm: Set encoder's mode of operation using a kms func
The mdp5 kms driver currently sets up multiple encoders per interface (INTF), one for each kind of mode of operation it supports. We create 2 drm_encoders for DSI, one for Video Mode and the other for Command Mode operation. The reason behind this approach could have been that we aren't aware of the DSI device's mode of operation when we create the encoders. This makes things a bit complicated, since these encoders have to be further attached to the same DSI bridge. The easier way out is to create a single encoder, and make the DSI driver set its mode of operation when we know what the DSI device's mode flags are. Start with providing a way to set the mdp5_intf_mode using a kms func that sets the encoder's mode of operation. When constructing a DSI encoder, we set the mode of operation to Video Mode as default. When the DSI device is attached to the host, we probe the DSI mode flags and set the corresponding mode of operation. Signed-off-by: Archit Taneja--- drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 2 ++ drivers/gpu/drm/msm/dsi/dsi_manager.c | 39 ++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 17 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 8 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 1 + drivers/gpu/drm/msm/msm_kms.h | 3 +++ 7 files changed, 68 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h index ddcda8cec9a7..81971b3caf3b 100644 --- a/drivers/gpu/drm/msm/dsi/dsi.h +++ b/drivers/gpu/drm/msm/dsi/dsi.h @@ -90,6 +90,7 @@ int msm_dsi_manager_phy_enable(int id, void msm_dsi_manager_phy_disable(int id); int msm_dsi_manager_cmd_xfer(int id, const struct mipi_dsi_msg *msg); bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len); +void msm_dsi_manager_attach_dsi_device(int id, u32 device_flags); int msm_dsi_manager_register(struct msm_dsi *msm_dsi); void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi); diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 3819fdefcae2..eb0903d37e5c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1482,6 +1482,8 @@ static int dsi_host_attach(struct mipi_dsi_host *host, msm_host->format = dsi->format; msm_host->mode_flags = dsi->mode_flags; + msm_dsi_manager_attach_dsi_device(msm_host->id, dsi->mode_flags); + /* Some gpios defined in panel DT need to be controlled by host */ ret = dsi_host_init_panel_gpios(msm_host, >dev); if (ret) diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c index 19da23d8a0b1..643a6df9a32c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c @@ -155,8 +155,16 @@ static enum drm_connector_status dsi_mgr_connector_detect( DBG("id=%d", id); if (!msm_dsi->panel) { + struct drm_encoder *encoder = msm_dsi_get_encoder(msm_dsi); + bool cmd_mode; + msm_dsi->panel = msm_dsi_host_get_panel(msm_dsi->host, - _dsi->device_flags); + _dsi->device_flags); + + cmd_mode = !(msm_dsi->device_flags & MIPI_DSI_MODE_VIDEO); + + if (kms->funcs->set_encoder_mode) + kms->funcs->set_encoder_mode(kms, encoder, cmd_mode); /* There is only 1 panel in the global panel list * for dual DSI mode. Therefore slave dsi should get @@ -177,8 +185,6 @@ static enum drm_connector_status dsi_mgr_connector_detect( */ if (msm_dsi->panel && IS_DUAL_DSI() && other_dsi && other_dsi->panel) { - bool cmd_mode = !(msm_dsi->device_flags & - MIPI_DSI_MODE_VIDEO); struct drm_encoder *encoder = msm_dsi_get_encoder( dsi_mgr_get_dsi(DSI_ENCODER_MASTER)); struct drm_encoder *slave_enc = msm_dsi_get_encoder( @@ -773,6 +779,33 @@ bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 dma_base, u32 len) return true; } +void msm_dsi_manager_attach_dsi_device(int id, u32 device_flags) +{ + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id); + struct drm_device *dev = msm_dsi->dev; + struct msm_drm_private *priv; + struct msm_kms *kms; + struct drm_encoder *encoder; + + /* +* drm_device pointer is assigned to msm_dsi only in the modeset_init +* path. If mipi_dsi_attach() happens in DSI driver's probe path +* (generally the case when we're connected to a drm_panel of the type +* mipi_dsi_device), this would be NULL. Try to set encoder mode in +
Request API: stateless VPU: the buffer mechanism and DPB management
Hello all: I have recently finish the learning of the H.264 codec and ready to write the driver. Although I have not get deep in syntax of H.264 but I think I just need to reuse and extended the VA-API H264 Parser from gstreamer. The whole plan in userspace is just injecting a parsing operation and set those v4l2 control in kernel before enqueue a buffer into OUTPUT, which would keep the most compatible with those stateful video IP(those with a firmware). But in order to do that, I can't avoid the management of DPB. I decided to moving the DPB management job from userspace in kernel. Also the video IP(On2 on rk3288 and the transition video IP on those future SoC than rk3288, rkv don't have this problem) would a special way to manage the DPB, which requests the same reference frame is storing in the same reference index in the runtime(actually it is its Motion Vector data appended in decoded YUV data would not be moved). I would suggest to keep those job in kernel, the userspace just to need update the list0 and list1 of DPB. DPB is self managed in kernel the userspace don't need to even dequeue the buffer from CAPTURE until the re-order is done. The kernel driver would also re-order the CAPTURE buffer into display order, and blocking the operation on CAPTURE until a buffer is ready to place in the very display order. If I don't do that, I have to get the buffer once it is decoded, and marking its result with the poc, I could only begin the processing of the next frame only after those thing are done. Which would effect the performance badly. That is what chromebook did(I hear that from the other staff, I didn't get invoke in chromium project yet). So I would suggest that doing the re-order job in kernel, and inform the the userspace the buffers are ready when the new I frame(key frame) is pushed into the video IP. Although moving those job into kernel would increase the loading, but I think it is worth to do that, but I don't know whether all those thought are correct and high performance(It is more important than API compatible especially for those 4K video). And I want to know more ideas about this topic. I would begin the writing the new driver after the coming culture new year vacation(I would go to the Europe), I wish we can decide the final mechanism before I begin this job. -- Randy Li The third produce department ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Request API: stateless VPU: the buffer mechanism and DPB management
Hello all: I have recently finish the learning of the H.264 codec and ready to write the driver. Although I have not get deep in syntax of H.264 but I think I just need to reuse and extended the VA-API H264 Parser from gstreamer. The whole plan in userspace is just injecting a parsing operation and set those v4l2 control in kernel before enqueue a buffer into OUTPUT, which would keep the most compatible with those stateful video IP(those with a firmware). But in order to do that, I can't avoid the management of DPB. I decided to moving the DPB management job from userspace in kernel. Also the video IP(On2 on rk3288 and the transition video IP on those future SoC than rk3288, rkv don't have this problem) would a special way to manage the DPB, which requests the same reference frame is storing in the same reference index in the runtime(actually it is its Motion Vector data appended in decoded YUV data would not be moved). I would suggest to keep those job in kernel, the userspace just to need update the list0 and list1 of DPB. DPB is self managed in kernel the userspace don't need to even dequeue the buffer from CAPTURE until the re-order is done. The kernel driver would also re-order the CAPTURE buffer into display order, and blocking the operation on CAPTURE until a buffer is ready to place in the very display order. If I don't do that, I have to get the buffer once it is decoded, and marking its result with the poc, I could only begin the processing of the next frame only after those thing are done. Which would effect the performance badly. That is what chromebook did(I hear that from the other staff, I didn't get invoke in chromium project yet). So I would suggest that doing the re-order job in kernel, and inform the the userspace the buffers are ready when the new I frame(key frame) is pushed into the video IP. Although moving those job into kernel would increase the loading, but I think it is worth to do that, but I don't know whether all those thought are correct and high performance(It is more important than API compatible especially for those 4K video). And I want to know more ideas about this topic. I would begin the writing the new driver after the coming culture new year vacation(I would go to the Europe), I wish we can decide the final mechanism before I begin this job. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 04/12] drm/mediatek: add shadow register support
Hi, YT: On Wed, 2017-01-11 at 14:51 +0800, YT Shen wrote: > We need to acquire mutex before using the resources, > and need to release it after finished. > So we don't need to write registers in the blanking period. > > Signed-off-by: YT ShenAcked-by: CK Hu > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 75 > - > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 25 +++ > drivers/gpu/drm/mediatek/mtk_drm_ddp.h | 2 + > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 + > 4 files changed, 74 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 01a21dd..b9b82e5 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -329,6 +329,42 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc > *mtk_crtc) > pm_runtime_put(drm->dev); > } > > +static void mtk_crtc_ddp_config(struct drm_crtc *crtc) > +{ > + struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > + struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state); > + struct mtk_ddp_comp *ovl = mtk_crtc->ddp_comp[0]; > + unsigned int i; > + > + /* > + * TODO: instead of updating the registers here, we should prepare > + * working registers in atomic_commit and let the hardware command > + * queue update module registers on vblank. > + */ > + if (state->pending_config) { > + mtk_ddp_comp_config(ovl, state->pending_width, > + state->pending_height, > + state->pending_vrefresh, 0); > + > + state->pending_config = false; > + } > + > + if (mtk_crtc->pending_planes) { > + for (i = 0; i < OVL_LAYER_NR; i++) { > + struct drm_plane *plane = _crtc->planes[i]; > + struct mtk_plane_state *plane_state; > + > + plane_state = to_mtk_plane_state(plane->state); > + > + if (plane_state->pending.config) { > + mtk_ddp_comp_layer_config(ovl, i, plane_state); > + plane_state->pending.config = false; > + } > + } > + mtk_crtc->pending_planes = false; > + } > +} > + > static void mtk_drm_crtc_enable(struct drm_crtc *crtc) > { > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > @@ -405,6 +441,7 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc > *crtc, > struct drm_crtc_state *old_crtc_state) > { > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > + struct mtk_drm_private *priv = crtc->dev->dev_private; > unsigned int pending_planes = 0; > int i; > > @@ -426,6 +463,12 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc > *crtc, > if (crtc->state->color_mgmt_changed) > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) > mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state); > + > + if (priv->data->shadow_register) { > + mtk_disp_mutex_acquire(mtk_crtc->mutex); > + mtk_crtc_ddp_config(crtc); > + mtk_disp_mutex_release(mtk_crtc->mutex); > + } > } > > static const struct drm_crtc_funcs mtk_crtc_funcs = { > @@ -471,36 +514,10 @@ static int mtk_drm_crtc_init(struct drm_device *drm, > void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *ovl) > { > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > - struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state); > - unsigned int i; > + struct mtk_drm_private *priv = crtc->dev->dev_private; > > - /* > - * TODO: instead of updating the registers here, we should prepare > - * working registers in atomic_commit and let the hardware command > - * queue update module registers on vblank. > - */ > - if (state->pending_config) { > - mtk_ddp_comp_config(ovl, state->pending_width, > - state->pending_height, > - state->pending_vrefresh, 0); > - > - state->pending_config = false; > - } > - > - if (mtk_crtc->pending_planes) { > - for (i = 0; i < OVL_LAYER_NR; i++) { > - struct drm_plane *plane = _crtc->planes[i]; > - struct mtk_plane_state *plane_state; > - > - plane_state = to_mtk_plane_state(plane->state); > - > - if (plane_state->pending.config) { > - mtk_ddp_comp_layer_config(ovl, i, plane_state); > - plane_state->pending.config = false; > - } > - } > - mtk_crtc->pending_planes = false; > - } > + if (!priv->data->shadow_register) > +
Re: [PATCH v11 03/12] drm/mediatek: add *driver_data for different hardware settings
Hi, YT: On Wed, 2017-01-11 at 14:51 +0800, YT Shen wrote: > There are some hardware settings changed, between MT8173 & MT2701: > DISP_OVL address offset changed, color format definition changed. > DISP_RDMA fifo size changed. > DISP_COLOR offset changed. > MIPI_TX pll setting changed. > And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod. > > Signed-off-by: YT ShenAcked-by: CK Hu > --- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 41 - > drivers/gpu/drm/mediatek/mtk_disp_rdma.c| 18 +++- > drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 71 > +++-- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 57 +++ > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 25 +++--- > drivers/gpu/drm/mediatek/mtk_drm_drv.h | 8 > drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 24 +- > 7 files changed, 181 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > index ce2759f..4552178 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > @@ -35,18 +35,27 @@ > #define DISP_REG_OVL_PITCH(n)(0x0044 + 0x20 * (n)) > #define DISP_REG_OVL_RDMA_CTRL(n)(0x00c0 + 0x20 * (n)) > #define DISP_REG_OVL_RDMA_GMC(n) (0x00c8 + 0x20 * (n)) > -#define DISP_REG_OVL_ADDR(n) (0x0f40 + 0x20 * (n)) > +#define DISP_REG_OVL_ADDR_MT8173 0x0f40 > +#define DISP_REG_OVL_ADDR(ovl, n)((ovl)->data->addr + 0x20 * (n)) > > #define OVL_RDMA_MEM_GMC0x40402020 > > #define OVL_CON_BYTE_SWAPBIT(24) > -#define OVL_CON_CLRFMT_RGB565(0 << 12) > -#define OVL_CON_CLRFMT_RGB888(1 << 12) > +#define OVL_CON_CLRFMT_RGB (1 << 12) > #define OVL_CON_CLRFMT_RGBA (2 << 12) > #define OVL_CON_CLRFMT_ARGB (3 << 12) > +#define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ > + 0 : OVL_CON_CLRFMT_RGB) > +#define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ > + OVL_CON_CLRFMT_RGB : 0) > #define OVL_CON_AEN BIT(8) > #define OVL_CON_ALPHA 0xff > > +struct mtk_disp_ovl_data { > + unsigned int addr; > + bool fmt_rgb565_is_0; > +}; > + > /** > * struct mtk_disp_ovl - DISP_OVL driver structure > * @ddp_comp - structure containing type enum and hardware resources > @@ -55,6 +64,7 @@ > struct mtk_disp_ovl { > struct mtk_ddp_comp ddp_comp; > struct drm_crtc *crtc; > + const struct mtk_disp_ovl_data *data; > }; > > static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) > @@ -141,18 +151,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp > *comp, unsigned int idx) > writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx)); > } > > -static unsigned int ovl_fmt_convert(unsigned int fmt) > +static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int > fmt) > { > switch (fmt) { > default: > case DRM_FORMAT_RGB565: > - return OVL_CON_CLRFMT_RGB565; > + return OVL_CON_CLRFMT_RGB565(ovl); > case DRM_FORMAT_BGR565: > - return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP; > + return OVL_CON_CLRFMT_RGB565(ovl) | OVL_CON_BYTE_SWAP; > case DRM_FORMAT_RGB888: > - return OVL_CON_CLRFMT_RGB888; > + return OVL_CON_CLRFMT_RGB888(ovl); > case DRM_FORMAT_BGR888: > - return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP; > + return OVL_CON_CLRFMT_RGB888(ovl) | OVL_CON_BYTE_SWAP; > case DRM_FORMAT_RGBX: > case DRM_FORMAT_RGBA: > return OVL_CON_CLRFMT_ARGB; > @@ -171,6 +181,7 @@ static unsigned int ovl_fmt_convert(unsigned int fmt) > static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx, >struct mtk_plane_state *state) > { > + struct mtk_disp_ovl *ovl = comp_to_ovl(comp); > struct mtk_plane_pending_state *pending = >pending; > unsigned int addr = pending->addr; > unsigned int pitch = pending->pitch & 0x; > @@ -182,7 +193,7 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp > *comp, unsigned int idx, > if (!pending->enable) > mtk_ovl_layer_off(comp, idx); > > - con = ovl_fmt_convert(fmt); > + con = ovl_fmt_convert(ovl, fmt); > if (idx != 0) > con |= OVL_CON_AEN | OVL_CON_ALPHA; > > @@ -190,7 +201,7 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp > *comp, unsigned int idx, > writel_relaxed(pitch, comp->regs + DISP_REG_OVL_PITCH(idx)); > writel_relaxed(src_size, comp->regs + DISP_REG_OVL_SRC_SIZE(idx)); > writel_relaxed(offset,
Re: [PATCH v3 0/7] drm/atomic: Add accessor macros for all atomic state.
Hi Maarten, One more thing. On Monday 16 Jan 2017 10:37:37 Maarten Lankhorst wrote: > Fourth iteration. Instead of trying to convert all drivers straight away, > implement all macros that are required to get state working. > > Old situation: > Use obj->state, which can refer to old or new state. > Use drm_atomic_get_(existing_)obj_state, which can refer to new or old > state. Use for_each_obj_in_state, which refers to new or old state. > > New situation: > > During atomic check: > - Use drm_atomic_get_obj_state to add a object to the atomic state, > or get the new state. > - Use drm_atomic_get_(old/new)_obj_state to peek at the new/old state, > without adding the object. This will return NULL if the object is > not part of the state. For planes and connectors the relevant crtc_state > is added, so this will work to get the crtc_state from obj_state->crtc > too, this means not having to write some error handling. > > During atomic commit: > - Do not use drm_atomic_get_obj_state, obj->state or > drm_atomic_get_(existing_)obj_state any more, replace with > drm_atomic_get_old/new_obj_state calls as required. There are four calls to the drm_atomic_get_existing_*_state() functions left in the DRM core after this series. There's also one call to drm_atomic_get_plane_state() in drm_blend.c. Could you please fix that ? > During both: > - Use for_each_(new,old,oldnew)_obj_in_state to get the old or new state as > needed. oldnew will be renamed to for_each_obj_in_state after all callers > are converted to the new api. > > When not doing atomic updates: > Look at obj->state for now. I have some patches to fix this but I was asked > to make it return a const state. This breaks a lot of users though so I > skipped that patch in this iteration. > > This series will return the correct state regardless of swapping. > > Maarten Lankhorst (7): > drm/atomic: Add new iterators over all state, v3. > drm/atomic: Make add_affected_connectors look at crtc_state. > drm/atomic: Use new atomic iterator macros. > drm/atomic: Fix atomic helpers to use the new iterator macros. > drm/atomic: Add macros to access existing old/new state > drm/atomic: Convert get_existing_state callers to get_old/new_state, v2. > drm/blend: Use new atomic iterator macros. > > drivers/gpu/drm/drm_atomic.c | 39 ++-- > drivers/gpu/drm/drm_atomic_helper.c | 377 +- > drivers/gpu/drm/drm_blend.c | 23 +-- > drivers/gpu/drm/i915/intel_display.c | 13 +- > include/drm/drm_atomic.h | 180 - > include/drm/drm_atomic_helper.h | 2 + > 6 files changed, 438 insertions(+), 196 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state, v2.
Hi Maarten, One more thing. On Monday 16 Jan 2017 10:37:43 Maarten Lankhorst wrote: > This is a straightforward conversion that converts all the users of > get_existing_state in atomic core to use get_old_state or get_new_state > > Changes since v1: > - Fix using the wrong state in > drm_atomic_helper_update_legacy_modeset_state. > > Signed-off-by: Maarten Lankhorst> --- > drivers/gpu/drm/drm_atomic.c| 6 +++--- > drivers/gpu/drm/drm_atomic_helper.c | 39 ++ > drivers/gpu/drm/drm_blend.c | 3 +-- > 3 files changed, 22 insertions(+), 26 deletions(-) [snip] > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index b26cf786ce12..1de8d5fbc8d3 > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c [snip] > @@ -698,8 +695,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) if (!old_conn_state->crtc) > continue; > > - old_crtc_state = drm_atomic_get_existing_crtc_state(old_state, > - old_conn_state->crtc); > + old_crtc_state = drm_atomic_get_new_crtc_state(old_state, > old_conn_state->crtc); This looks wrong. I believe you should call drm_atomic_get_old_crtc_state() here. If I'm not mistaken drm_atomic_get_existing_crtc_state() did the right thing as old_state->crtcs[*].state was set to the old state by the state swap operation. On the other hand, drm_atomic_helper_update_legacy_modeset_state() uses drm_atomic_get_new_plane_state() the same way you do here, even though it operates after state swap, and your changelog specifically mentions that you've fixed that in v2. It's getting too late to properly wrap my head around this, I'll let you check which option is correct (but I reserve the right to challenge it if your explanation isn't convincing enough ;-)). > if (!old_crtc_state->active || > !drm_atomic_crtc_needs_modeset(old_conn_state->crtc- >state)) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 7/7] drm/blend: Use new atomic iterator macros.
Hi Maarten, Thank you for the patch. On Monday 16 Jan 2017 10:37:44 Maarten Lankhorst wrote: Could we please get a commit message ? Apart from that, Reviewed-by: Laurent Pinchart> Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/drm_blend.c | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index 5c45d0852608..78cf9f6cae08 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -378,26 +378,26 @@ int drm_atomic_normalize_zpos(struct drm_device *dev, > struct drm_atomic_state *state) > { > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *old_plane_state, *new_plane_state; > int i, ret = 0; > > - for_each_plane_in_state(state, plane, plane_state, i) { > - crtc = plane_state->crtc; > + for_each_oldnew_plane_in_state(state, plane, old_plane_state, > new_plane_state, i) { > + crtc = new_plane_state->crtc; > if (!crtc) > continue; > - if (plane->state->zpos != plane_state->zpos) { > - crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > - crtc_state->zpos_changed = true; > + if (old_plane_state->zpos != new_plane_state->zpos) { > + new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc); > + new_crtc_state->zpos_changed = true; > } > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > - if (crtc_state->plane_mask != crtc->state->plane_mask || > - crtc_state->zpos_changed) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > + if (old_crtc_state->plane_mask != new_crtc_state->plane_mask > || > + new_crtc_state->zpos_changed) { > ret = drm_atomic_helper_crtc_normalize_zpos(crtc, > - crtc_state); > + new_crtc_state); > if (ret) > return ret; > } -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state, v2.
Hi Maarten, Thank you for the patch. I don't think you need the "v2" at the end of the subject line. On Monday 16 Jan 2017 10:37:43 Maarten Lankhorst wrote: > This is a straightforward conversion that converts all the users of > get_existing_state in atomic core to use get_old_state or get_new_state > > Changes since v1: > - Fix using the wrong state in > drm_atomic_helper_update_legacy_modeset_state. > > Signed-off-by: Maarten Lankhorst> --- > drivers/gpu/drm/drm_atomic.c| 6 +++--- > drivers/gpu/drm/drm_atomic_helper.c | 39 +++-- > drivers/gpu/drm/drm_blend.c | 3 +-- > 3 files changed, 22 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 7b61e0da9ace..6428e9469607 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c [snip] > @@ -1835,7 +1832,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct > drm_crtc_state *old_crtc_state) > > drm_for_each_plane_mask(plane, crtc->dev, plane_mask) { > struct drm_plane_state *old_plane_state = > - drm_atomic_get_existing_plane_state(old_state, plane); > + drm_atomic_get_old_plane_state(old_state, plane); I believe this change to be correct, but given that the function is called after state swap, didn't it use the new state before this patch ? > const struct drm_plane_helper_funcs *plane_funcs; > > plane_funcs = plane->helper_private; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 5/7] drm/atomic: Add macros to access existing old/new state
Hi Maarten, Thank you for the patch. On Monday 16 Jan 2017 10:37:42 Maarten Lankhorst wrote: > After atomic commit, these macros should be used in place of > get_existing_state. Also after commit get_xx_state should no longer > be used because it may not have the required locks. Should this be captured in the functions' documentation ? In particular, should the drm_atomic_get_existing_*_state() functions be marked as deprecated ? > Signed-off-by: Maarten Lankhorst> --- > include/drm/drm_atomic.h | 99 > 1 file changed, 99 insertions(+) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 6062e7f27325..2e6bb7acc837 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -285,6 +285,35 @@ drm_atomic_get_existing_crtc_state(struct > drm_atomic_state *state, } > > /** > + * drm_atomic_get_old_crtc_state - get old crtc state, if it exists > + * @state: global atomic state object > + * @crtc: crtc to grab > + * > + * This function returns the old crtc state for the given crtc, or > + * NULL if the crtc is not part of the global atomic state. > + */ > +static inline struct drm_crtc_state * > +drm_atomic_get_old_crtc_state(struct drm_atomic_state *state, > + struct drm_crtc *crtc) > +{ > + return state->crtcs[drm_crtc_index(crtc)].old_state; > +} > +/** > + * drm_atomic_get_new_crtc_state - get new crtc state, if it exists > + * @state: global atomic state object > + * @crtc: crtc to grab > + * > + * This function returns the new crtc state for the given crtc, or > + * NULL if the crtc is not part of the global atomic state. > + */ > +static inline struct drm_crtc_state * > +drm_atomic_get_new_crtc_state(struct drm_atomic_state *state, > + struct drm_crtc *crtc) > +{ > + return state->crtcs[drm_crtc_index(crtc)].new_state; > +} > + > +/** > * drm_atomic_get_existing_plane_state - get plane state, if it exists > * @state: global atomic state object > * @plane: plane to grab > @@ -300,6 +329,36 @@ drm_atomic_get_existing_plane_state(struct > drm_atomic_state *state, } > > /** > + * drm_atomic_get_old_plane_state - get plane state, if it exists > + * @state: global atomic state object > + * @plane: plane to grab > + * > + * This function returns the old plane state for the given plane, or > + * NULL if the plane is not part of the global atomic state. > + */ > +static inline struct drm_plane_state * > +drm_atomic_get_old_plane_state(struct drm_atomic_state *state, > +struct drm_plane *plane) > +{ > + return state->planes[drm_plane_index(plane)].old_state; > +} > + > +/** > + * drm_atomic_get_new_plane_state - get plane state, if it exists > + * @state: global atomic state object > + * @plane: plane to grab > + * > + * This function returns the new plane state for the given plane, or > + * NULL if the plane is not part of the global atomic state. > + */ > +static inline struct drm_plane_state * > +drm_atomic_get_new_plane_state(struct drm_atomic_state *state, > +struct drm_plane *plane) > +{ > + return state->planes[drm_plane_index(plane)].new_state; > +} > + > +/** > * drm_atomic_get_existing_connector_state - get connector state, if it > exists * @state: global atomic state object > * @connector: connector to grab > @@ -320,6 +379,46 @@ drm_atomic_get_existing_connector_state(struct > drm_atomic_state *state, } > > /** > + * drm_atomic_get_old_connector_state - get connector state, if it exists > + * @state: global atomic state object > + * @connector: connector to grab > + * > + * This function returns the old connector state for the given connector, > + * or NULL if the connector is not part of the global atomic state. > + */ > +static inline struct drm_connector_state * > +drm_atomic_get_old_connector_state(struct drm_atomic_state *state, > +struct drm_connector *connector) > +{ > + int index = drm_connector_index(connector); > + > + if (index >= state->num_connector) > + return NULL; > + > + return state->connectors[index].old_state; > +} > + > +/** > + * drm_atomic_get_new_connector_state - get connector state, if it exists > + * @state: global atomic state object > + * @connector: connector to grab > + * > + * This function returns the new connector state for the given connector, > + * or NULL if the connector is not part of the global atomic state. > + */ > +static inline struct drm_connector_state * > +drm_atomic_get_new_connector_state(struct drm_atomic_state *state, > +struct drm_connector *connector) > +{ > + int index = drm_connector_index(connector); > + > + if (index >= state->num_connector) > + return NULL; > + > + return state->connectors[index].new_state; > +} > + > +/** > * __drm_atomic_get_current_plane_state -
Re: [PATCH v3 4/7] drm/atomic: Fix atomic helpers to use the new iterator macros.
Hi Maarten, (CC'ing Daniel) Thank you for the patch. On Monday 16 Jan 2017 10:37:41 Maarten Lankhorst wrote: > Signed-off-by: Maarten Lankhorst> --- > drivers/gpu/drm/drm_atomic_helper.c | 293 +--- > 1 file changed, 152 insertions(+), 141 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index d153e8a72921..b26cf786ce12 > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c [snip] > @@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state, > { > struct drm_crtc_state *crtc_state; > struct drm_connector *connector; > - struct drm_connector_state *connector_state; > + struct drm_connector_state *old_connector_state, *new_connector_state; The kernel favours one variable declaration per line, and I think this file does as well, at least mostly (this comment holds for the rest of this patch and the other patches in the series). > int i; > > - for_each_connector_in_state(state, connector, connector_state, i) { > + for_each_oldnew_connector_in_state(state, connector, > old_connector_state, new_connector_state, i) { This is getting quite long, you could wrap the line. > struct drm_crtc *encoder_crtc; > > - if (connector_state->best_encoder != encoder) > + if (new_connector_state->best_encoder != encoder) > continue; > > - encoder_crtc = connector->state->crtc; > + encoder_crtc = old_connector_state->crtc; > > DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], stealing it\n", > encoder->base.id, encoder->name, >encoder_crtc->base.id, encoder_crtc->name); > > - set_best_encoder(state, connector_state, NULL); > + set_best_encoder(state, new_connector_state, NULL); > > crtc_state = drm_atomic_get_existing_crtc_state(state, encoder_crtc); > crtc_state->connectors_changed = true; [snip] > @@ -516,14 +518,15 @@ drm_atomic_helper_check_modeset(struct drm_device > *dev, if (ret) > return ret; > > - for_each_connector_in_state(state, connector, connector_state, i) { > + for_each_oldnew_connector_in_state(state, connector, > old_connector_state, new_connector_state, i) { Line wrap here too ? > /* >* This only sets crtc->connectors_changed for routing changes, >* drivers must set crtc->connectors_changed themselves when >* connector properties need to be updated. >*/ > ret = update_connector_routing(state, connector, > -connector_state); > +old_connector_state, > +new_connector_state); > if (ret) > return ret; > } [snip] > @@ -599,15 +602,15 @@ drm_atomic_helper_check_planes(struct drm_device *dev, > struct drm_crtc *crtc; > struct drm_crtc_state *crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *plane_state, *old_plane_state; In some location you use new_*_state and in others *_state. I believe this should this be standardized to avoid confusion (the code is hard enough to read as it is :-)). Similarly, you sometimes use *_conn_state and sometimes *_connector_state. That should be standardized as well. > int i, ret = 0; > > - for_each_plane_in_state(state, plane, plane_state, i) { > + for_each_oldnew_plane_in_state(state, plane, old_plane_state, > plane_state, i) { > const struct drm_plane_helper_funcs *funcs; > > funcs = plane->helper_private; > > - drm_atomic_helper_plane_changed(state, plane_state, plane); > + drm_atomic_helper_plane_changed(state, old_plane_state, > plane_state, plane); > > if (!funcs || !funcs->atomic_check) > continue; [snip] > @@ -974,18 +977,18 @@ void drm_atomic_helper_commit_modeset_enables(struct > drm_device *dev, } > } > > - for_each_connector_in_state(old_state, connector, old_conn_state, i) { > + for_each_new_connector_in_state(old_state, connector, conn_state, i) { Not strictly related to this patch, but I've been wondering how this works, given that we need to loop over connectors in the new state, not the old state. After some investigation I've realized that both the old and new states contain the same list of objects, as we don't keep a copy of the old global atomic state. old_state was the new state before the state swap, and the swap operation sets state->/objects/[*].state to the old state for all objects, but doesn't modify the object pointers. This is possibly more of a
linux-next: manual merge of the drm tree with the drm-misc-fixes tree
Hi Dave, Today's linux-next merge of the drm tree got a conflict in: include/drm/drm_atomic.h between commit: 7e9081c5aac7 ("drm/fence: fix memory overwrite when setting out_fence fd") from the drm-misc-fixes tree and commit: bdc571464c49 ("drm/atomic: Clean up wait_for_vblanks, v2.") from the drm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc include/drm/drm_atomic.h index 56814e8ae7ea,f96220ed4004.. --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@@ -144,7 -144,8 +144,8 @@@ struct __drm_crtcs_state struct drm_crtc *ptr; struct drm_crtc_state *state; struct drm_crtc_commit *commit; - s64 __user *out_fence_ptr; + s32 __user *out_fence_ptr; + unsigned last_vblank_count; }; struct __drm_connnectors_state { ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/bridge: adv7511: Enable HPD interrupts to support hotplug and improve monitor detection
From: Archit TanejaOn some adv7511 implementations, we can get some spurious disconnect signals which can cause monitor probing to fail. This patch enables HPD (hot plug detect) interrupt support which allows the monitor to be properly re-initialized when the spurious disconnect signal goes away. This also enables proper hotplug support. Cc: David Airlie Cc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Acked-by: Laurent Pinchart Tested-by: Laurent Pinchart Originally-by: Archit Taneja [jstultz: Added proper commit message] Signed-off-by: John Stultz --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 7b2b5af..405e460 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -338,7 +338,7 @@ static void adv7511_power_on(struct adv7511 *adv7511) * Still, let's be safe and stick to the documentation. */ regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), -ADV7511_INT0_EDID_READY); +ADV7511_INT0_EDID_READY | ADV7511_INT0_HPD); regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), ADV7511_INT1_DDC_ERROR); } @@ -846,6 +846,10 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge) if (adv->type == ADV7533) ret = adv7533_attach_dsi(adv); + if (adv->i2c_main->irq) + regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0), + ADV7511_INT0_HPD); + return ret; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] drm/bridge: adv7511: Switch to using drm_kms_helper_hotplug_event()
In chasing down a previous issue with EDID probing from calling drm_helper_hpd_irq_event() from irq context, Laurent noticed that the DRM documentation suggests that drm_kms_helper_hotplug_event() should be used instead. Thus this patch replaces drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event(), which requires we update the connector.status entry and only call _hotplug_event() when the status changes. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Reviewed-by: Laurent Pinchart Tested-by: Laurent Pinchart Signed-off-by: John Stultz --- v3: Update connector.status value and only call __hotplug_event() when that status changes, as suggested by Laurent. v4: Set adv7511->connector.status before calling adv7511->connector.status, as suggested by Laurent. --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 4fcea44..7b2b5af 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -405,8 +405,22 @@ static bool adv7511_hpd(struct adv7511 *adv7511) static void adv7511_hpd_work(struct work_struct *work) { struct adv7511 *adv7511 = container_of(work, struct adv7511, hpd_work); + enum drm_connector_status status; + unsigned int val; + int ret; - drm_helper_hpd_irq_event(adv7511->connector.dev); + ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, ); + if (ret < 0) + status = connector_status_disconnected; + else if (val & ADV7511_STATUS_HPD) + status = connector_status_connected; + else + status = connector_status_disconnected; + + if (adv7511->connector.status != status) { + adv7511->connector.status = status; + drm_kms_helper_hotplug_event(adv7511->connector.dev); + } } static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally
In chasing down issues with EDID probing, I found some duplicated but incomplete logic used to power the chip on and off. This patch refactors the adv7511_power_on/off functions, so they can be used for internal needs. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- v4: Move the regcache_sync() call outside the shared implementation as we don't want to call it on EDID probing, as suggested by Laurent --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 405e460..12f2876 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -325,7 +325,7 @@ static void adv7511_set_link_config(struct adv7511 *adv7511, adv7511->rgb = config->input_colorspace == HDMI_COLORSPACE_RGB; } -static void adv7511_power_on(struct adv7511 *adv7511) +static void __adv7511_power_on(struct adv7511 *adv7511) { adv7511->current_edid_segment = -1; @@ -354,6 +354,11 @@ static void adv7511_power_on(struct adv7511 *adv7511) regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER2, ADV7511_REG_POWER2_HPD_SRC_MASK, ADV7511_REG_POWER2_HPD_SRC_NONE); +} + +static void adv7511_power_on(struct adv7511 *adv7511) +{ + __adv7511_power_on(adv7511); /* * Most of the registers are reset during power down or when HPD is low. @@ -362,21 +367,23 @@ static void adv7511_power_on(struct adv7511 *adv7511) if (adv7511->type == ADV7533) adv7533_dsi_power_on(adv7511); - adv7511->powered = true; } -static void adv7511_power_off(struct adv7511 *adv7511) +static void __adv7511_power_off(struct adv7511 *adv7511) { /* TODO: setup additional power down modes */ regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, ADV7511_POWER_POWER_DOWN, ADV7511_POWER_POWER_DOWN); regcache_mark_dirty(adv7511->regmap); +} +static void adv7511_power_off(struct adv7511 *adv7511) +{ + __adv7511_power_off(adv7511); if (adv7511->type == ADV7533) adv7533_dsi_power_off(adv7511); - adv7511->powered = false; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID
Thus this patch changes the EDID probing logic so that we re-use the __adv7511_power_on/off() calls instead of duplciating logic. This does change behavior slightly as it adds the HPD signal pulse to the EDID probe path, but Archit has had a patch to add HPD signal pulse to the EDID probe path before, so this should address the cases where that helped. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- v4: Reworded commit message, focusing on re-use and HPD pulse behavior change. --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 17 +++-- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 12f2876..d216f61 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -573,24 +573,13 @@ static int adv7511_get_modes(struct adv7511 *adv7511, unsigned int count; /* Reading the EDID only works if the device is powered */ - if (!adv7511->powered) { - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, - ADV7511_POWER_POWER_DOWN, 0); - if (adv7511->i2c_main->irq) { - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(0), -ADV7511_INT0_EDID_READY); - regmap_write(adv7511->regmap, ADV7511_REG_INT_ENABLE(1), -ADV7511_INT1_DDC_ERROR); - } - adv7511->current_edid_segment = -1; - } + if (!adv7511->powered) + __adv7511_power_on(adv7511); edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); if (!adv7511->powered) - regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, - ADV7511_POWER_POWER_DOWN, - ADV7511_POWER_POWER_DOWN); + __adv7511_power_off(adv7511); kfree(adv7511->edid); adv7511->edid = edid; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] drm/bridge: adv7511: Re-write the i2c address before EDID probing
I've found that by just turning the chip on and off via the POWER_DOWN register, I end up getting i2c_transfer errors on HiKey. Investigating further, it seems some of the register state in the regmap cache is getting lost, likely as the device registers were reset during power off. Thus this patch simply re-writes the i2c address to the ADV7511_REG_EDID_I2C_ADDR register to ensure its properly set before we try to read the EDID data. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- v4: New approach to make the EDID_I2C_ADDR register sane, as suggested by Laurent --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index d216f61..0ed89ea 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -573,9 +573,17 @@ static int adv7511_get_modes(struct adv7511 *adv7511, unsigned int count; /* Reading the EDID only works if the device is powered */ - if (!adv7511->powered) + if (!adv7511->powered) { + unsigned int edid_i2c_addr = + (adv7511->i2c_main->addr << 1) + 4; + __adv7511_power_on(adv7511); + /* Reset the EDID_I2C_ADDR register as it might be cleared */ + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, +edid_i2c_addr); + } + edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); if (!adv7511->powered) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6 v4] adv7511 EDID probing improvements
Wanted to re-send out v4 of this patch set, integrating some changes suggested by Laurent, for consideration for merging for v4.11 The first three patches are fixups that are hopefully straight forward, integrating feedback I got from Laurant, with minimal change from the previous versions. The last three patches try to clean up and rework the EDID probing code, to avoid issues seen on HiKey. I've reworked these more significantly since v3 to address feedback from Laurent. Thoughts and feedback would be appreciated! thanks -john New in v4: * Tweaked connector.status assignment to avoid race, as suggested by Laurent * Reworked the __adv7511_power_on helpers to avoid calling regcache_sync in the EDID probe path * Added new patch to set EDID_I2C_ADDR register before doing EDID read. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Archit Taneja (1): drm/bridge: adv7511: Enable HPD interrupts to support hotplug and improve monitor detection John Stultz (5): drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context drm/bridge: adv7511: Switch to using drm_kms_helper_hotplug_event() drm/bridge: adv7511: Rework adv7511_power_on/off() so they can be reused internally drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID drm/bridge: adv7511: Re-write the i2c address before EDID probing drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 + drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 67 2 files changed, 51 insertions(+), 18 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context
I was recently seeing issues with EDID probing, where the logic to wait for the EDID read bit to be set by the IRQ wasn't happening and the code would time out and fail. Digging deeper, I found this was due to the fact that IRQs were disabled as we were running in IRQ context from the HPD signal. Thus this patch changes the logic to handle the HPD signal via a work_struct so we can be out of irq context. With this patch, the EDID probing on hotplug does not time out. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Reviewed-by: Laurent Pinchart Tested-by: Laurent Pinchart Signed-off-by: John Stultz --- v3: Rename irq_work to hpd_work and remove extra whitespace, as suggested by Laurent --- drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 ++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 11 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..0396791 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -317,6 +317,8 @@ struct adv7511 { bool edid_read; wait_queue_head_t wq; + struct work_struct hpd_work; + struct drm_bridge bridge; struct drm_connector connector; diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..4fcea44 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -402,6 +402,13 @@ static bool adv7511_hpd(struct adv7511 *adv7511) return false; } +static void adv7511_hpd_work(struct work_struct *work) +{ + struct adv7511 *adv7511 = container_of(work, struct adv7511, hpd_work); + + drm_helper_hpd_irq_event(adv7511->connector.dev); +} + static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) { unsigned int irq0, irq1; @@ -419,7 +426,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd) regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1); if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder) - drm_helper_hpd_irq_event(adv7511->connector.dev); + schedule_work(>hpd_work); if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) { adv7511->edid_read = true; @@ -1006,6 +1013,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) goto err_i2c_unregister_edid; } + INIT_WORK(>hpd_work, adv7511_hpd_work); + if (i2c->irq) { init_waitqueue_head(>wq); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 0/3] Generic HDMI codec: Add channel mapping control
Hello, Any comments on this patch-set? what about the introduction of pcm new callback in DAI ops to use chmap helpers? Is it something reasonable, or should i come back on V1? Regards Arnaud > 2017-01-03 16:52 GMT+01:00 Arnaud Pouliquen: > Aim of this patch is to add 'Playback Channel Map' control to export > audio capabilities in term of HDMI sink speakers allocation. > > V4: > Abandon "Generic HDMI codec: Add channel mapping control" patch as it > generates warnings during compilation. > > Workaround is to define 2 constant tables in hdmi-codec.c to declare channel > mapping. > One for stereo and one for multichannel. > Consequence is that the behaviour is changed: >The chmap multichannel table export the HDMI CA configuration (tlv) and > not only the one suuported by HDMI sink. > Furthermore the chmap control .get handler is overwritten to allow to export > to user the selected configuration. > > - "ASoC: hdmi-codec: add channel mapping control": > - add hdmi_codec_stereo_chmaps and hdmi_codec_8ch_chmaps tables. > - implement chmap control get handler. > - "DRM: add help to get ELD speaker allocation" > => No delta vs V2. > - "ALSA pcm: allow non constant snd_pcm_chmap_elem" > => abandonned > - "ASoC: core: add optional pcm_new callback for DAI driver" > => No delta vs V2. > > V3: > - "ASoC: hdmi-codec: add channel mapping control": > => Minor fixes: > - select stereo speaker config by default if HDMI cable unplugged > - fix compilation warning. > - "DRM: add help to get ELD speaker allocation" > => No delta vs V2. > - "ALSA pcm: allow non constant snd_pcm_chmap_elem" > => No delta vs V2. > - "ASoC: core: add optional pcm_new callback for DAI driver" > => No delta vs V2. > > V2: > In this version I use chmap helper functions from pcm_lib.c. > It is quite tricky to use it from ASoC due to the relation chip of the > controls > with the pcm runtime. > After several tries, my conclusion is that it must be handled in ASoC DAI > driver. > Main reason is that the DAI driver can not provide snd_pcm_chmap struct > through kcontrol structure. But this induces that soc-core provides pcm > runtime > structure to DAI driver during probe. > > Base on this conclusion. I reworked my patches by adding 2 > new patches in patch-set > 1) ALSA pcm: allow non constant snd_pcm_chmap_elem >This patch allows to handle non constant channel mapping. As ELD >information can change during runtime it is mandatory to properly >handle the feature. >In term of compatibility with legacy it should be straightforward, >as update just consists in suppressing the 'const' constraint. > > 2) ASoC: core: add optional pcm_new callback for DAI driver > This is the only way i found to be able to use > snd_pcm_add_chmap_ctls and associated controls helper functions. > From my point of view, this is the more simple way to add relationship > between DAI and associated pcm devices. >Notice that this patch, if accepted, makes the following one obsolete, >as it also answer to the associated topic: > [PATCH v5 0/2] ALSA controls management using index/device/sub-devices > fields > (http://www.spinics.net/lists/alsa-devel/msg57639.html). > > If you estimate that this it not reasonable i will come back to my first > version. > > V1: > This patch follows discussion initiate here: > [RFC] ASOC: HDMI audio info frame speaker allocation > http://www.spinics.net/lists/alsa-devel/msg57363.html > > The code is fully inspired from HDA driver. > On hw_param, HDMI sink speaker capabilities are exported via TLV ops > and a CEA allocation is choson, based on ELD information and the number of > channels requested by user. > > Mains differences with HDA implementation are: > - Control is read only > - Channel swap is not supported. Consequence is that unused channel in >the mid of CEA audio infoframe channel mapping are considered as >active. >example for channel allocation 0x02: FL, FR, 0, FC) > This configuration is only available for a 4 channels stream. > - Channel allocation table has been reordered and HDMI 2.0 is not > supported. > > Arnaud Pouliquen (4): > DRM: add help to get ELD speaker allocation > ASoC: core: add optional pcm_new callback for DAI driver > ASoC: hdmi-codec: add channel mapping control > > include/drm/drm_edid.h| 13 ++ > include/sound/soc-dai.h | 3 + > sound/soc/codecs/hdmi-codec.c | 380 > +- > sound/soc/soc-core.c | 28 > 4 files changed, 423 insertions(+), 1 deletion(-) > > -- > 1.9.1 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH V7 1/4] Documentation/devicetree/bindings: b850v3_lvds_dp
On Tue, Jan 10, 2017 at 11:04:58PM +0200, Laurent Pinchart wrote: > Hi Peter, Laurent! > > On Saturday 07 Jan 2017 01:29:52 Peter Senna Tschudin wrote: > > On 04 January, 2017 21:39 CET, Rob Herring wrote: > > > On Tue, Jan 3, 2017 at 5:34 PM, Peter Senna Tschudin wrote: > > >> On 03 January, 2017 23:51 CET, Rob Herringwrote: > > >>> On Sun, Jan 01, 2017 at 09:24:29PM +0100, Peter Senna Tschudin wrote: > > Devicetree bindings documentation for the GE B850v3 LVDS/DP++ > > display bridge. > > > > Cc: Martyn Welch > > Cc: Martin Donnelly > > Cc: Javier Martinez Canillas > > Cc: Enric Balletbo i Serra > > Cc: Philipp Zabel > > Cc: Rob Herring > > Cc: Fabio Estevam > > Signed-off-by: Peter Senna Tschudin > > --- > > There was an Acked-by from Rob Herring for V6, but > > I changed the bindings to use i2c_new_secondary_device() so I > > removed it from the commit message. > > > > .../devicetree/bindings/ge/b850v3-lvds-dp.txt | 39 ++ > > >>> Generally, bindings are not organized by vendor. Put in > > >>> bindings/display/bridge/... instead. > > >> > > >> Will change that. > > >> > > 1 file changed, 39 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt > > > > diff --git a/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt > > b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt new file > > mode 100644 > > index 000..1bc6ebf > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt > > @@ -0,0 +1,39 @@ > > +Driver for GE B850v3 LVDS/DP++ display bridge > > + > > +Required properties: > > + - compatible : should be "ge,b850v3-lvds-dp". > > >>> > > >>> Isn't '-lvds-dp' redundant? The part# should be enough. > > >> > > >> b850v3 is the name of the product, this is why the proposed name. What > > >> about, b850v3-dp2 dp2 indicating the second DP output? > > > > > > Humm, b850v3 is the board name? This node should be the name of the bridge > > > chip. > > > > From the cover letter: > > > > -- // -- > > There are two physical bridges on the video signal pipeline: a STDP4028(LVDS > > to DP) and a STDP2690(DP to DP++). The hardware and firmware made it > > complicated for this binding to comprise two device tree nodes, as the > > design goal is to configure both bridges based on the LVDS signal, which > > leave the driver powerless to control the video processing pipeline. The > > two bridges behaves as a single bridge, and the driver is only needed for > > telling the host about EDID / HPD, and for giving the host powers to ack > > interrupts. The video signal pipeline is as follows: > > > > Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output > > -- // -- > > You forgot to prefix your patch series with [HACK] ;-) > > How about fixing the issues that make the two DT nodes solution difficult ? > What are they ? The Firmware and the hardware design. Both bridges, with stock firmware, are fully capable of providig EDID information and handling interrupts. But on this specific design, with this specific firmware, I need to read EDID from one bridge, and handle interrupts on the other. Back when I was starting the development I could not come up with a proper way to split EDID and interrupts between two bridges in a way that would result in a fully functional connector. Did I miss something? > > -- > Regards, > > Laurent Pinchart > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] ARM: EXYNOS: Remove Exynos4415 driver (SoC not supported anymore)
On 2017년 01월 14일 21:36, Krzysztof Kozlowski wrote: > Support for Exynos4415 is going away because there are no internal nor > external users. > > Since commit 46dcf0ff0de3 ("ARM: dts: exynos: Remove exynos4415.dtsi"), > the platform cannot be instantiated so remove also the mach code. > > Signed-off-by: Krzysztof Kozlowski> --- > arch/arm/mach-exynos/Kconfig | 5 - > arch/arm/mach-exynos/exynos.c | 1 - > arch/arm/mach-exynos/suspend.c | 1 - > 3 files changed, 7 deletions(-) Reviewed-by: Chanwoo Choi [snip] -- Best Regards, Chanwoo Choi S/W Center, Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: bridge: dw-hdmi: Define and use macros for PHY register addresses
Hi Laurent, 在 2017年01月13日 21:46, Laurent Pinchart 写道: On Friday 13 Jan 2017 09:32:54 Nickey.Yang wrote: 在 2017年01月12日 23:26, Laurent Pinchart 写道: On Thursday 12 Jan 2017 09:45:31 Nickey.Yang wrote: 在 2017年01月12日 07:49, Laurent Pinchart 写道: Replace the hardcoded register address numerical values with macros to clarify the code. This change has been tested by comparing the assembly code before and after the change. Signed-off-by: Laurent Pinchart--- drivers/gpu/drm/bridge/dw-hdmi.c | 35 - drivers/gpu/drm/bridge/dw-hdmi.h | 66 + 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/bridge/dw-hdmi.h b/drivers/gpu/drm/bridge/dw-hdmi.h index a4fd64a203c9..f3c149c88d71 100644 --- a/drivers/gpu/drm/bridge/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/dw-hdmi.h @@ -1085,4 +1085,70 @@ enum { HDMI_I2CM_CTLINT_ARB_MASK = 0x4, }; +/* + * HDMI 3D TX PHY registers + */ Why is there 3D related words? The register names come from the i.MX6 datasheet. I don't have access to any PHY datasheet from Synopsys, and the Rockchip RK3288 manuals I've been able to find don't document the HDMI PHY register. As the PHY used by i.MX6 is a "DWC HDMI 3D TX PHY", I'd used that name in the code. The "DWC MHL PHY" used by RK3288 seems to be use a similar (if not identical) registers map, at least for the registers configured by the driver. The "DWC HDMI 2.0 TX PHY" PHY used by R-Car Gen3, however, seems not to have a compatible register interface, except for the HDMI_3D_TX_PHY_PTRPT_ENBL register. I'm open to suggestions for a better naming scheme. If you have additional information that I don't have access to, please feel free to use them to propose improvements or to point out my mistakes :-) I did not find "3D" words in PHY IP vendor document. How is the PHY named in the documentation you have access to ? just remove 3D word,such as name HDMI_TX_PHY_PWRCTRL instead of HDMI_3D_TX_PHY_PWRCTRL in doc: HDMI Tx PHY for GLOBAL FOUNDRIES 28-nm SLP/1.8 V Databook, LCA Edition 11.1 Control Registers Module Design Architecture https://www.synopsys.com/dw/ipdir.php?ds=dwc_hdmi_14_csds_tx doesn't list that document, but mentions "DesignWare Cores HDMI 3D TX PHY for GLOBALFOUNDARIES 28-nm SLP/1.8 V Databook". The description of the corresponding PHY, however, is "HDMI 1.4 TX PHY 3.4Gbps in GF 28nm SLP 1.8V, East/West Poly Orientation". I assume that HDMI 1.4 TX PHYs originally didn't have 3D support, and new PHYs with 3D support were then named "HDMI 3D TX PHY". After some time PHYs without 3D support were dropped from the product line, and 3D then got dropped from the name. I agree with your point, I have compared the whole of registers, most of them is the same. This patch should not affect use, so i think weather the 3D words are reserved or not is OK. We could use the "HDMI_14_TX_PHY" prefix, to differentiate the registers from the HDMI 2.0 TX PHY registers that would use a "HDMI_20_TX_PHY" prefix. "HDMI_TX_PHY" prefix could be used for registers common between the two PHY types. I'm sure that wouldn't cover the whole product range, but with no additional information it's hard to come up with a better proposal (actually even this wouldn't be very easy, as I don't have much documentation about the HDMI 2.0 TX PHY registers). By the way, does your HDMI TX controller databook (not the TX PHY but the TX controller) mention "HDMI 3D TX PHY" in the config2_id register (0x0006) documentation ? How does it name PHY type 0xF2 for instance ? And PHY type 0xC2, which I believe is the PHY used by the RK3288 (please correct me if I'm wrong there) ? Configuration Identification Register 2(0x0006) Bits Name Description 7:0 phytype Indicates the type of PHY interface selected: 0x00: Legacy PHY (HDMI TX PHY) 0xB2: PHY_HDMI-MHL with HEAC 0xC2: PHY_HDMI-MHL 0xF2: PHY_GEN2 (HDMI 3D TX PHY) 0xE2: PHY_GEN2 (HDMI 3D TX PHY) + HEAC PHY and in rk3288 root@linaro-alip:~# io -4 0xff980018 ff980018: 00c2 RK3288 use "PHY_HDMI-MHL" +#define HDMI_3D_TX_PHY_PWRCTRL 0x00 +#define HDMI_3D_TX_PHY_SERDIVCTRL 0x01 +#define HDMI_3D_TX_PHY_SERCKCTRL 0x02 +#define HDMI_3D_TX_PHY_SERCKKILLCTRL 0x03 +#define HDMI_3D_TX_PHY_TXRESCTRL 0x04 +#define HDMI_3D_TX_PHY_CKCALCTRL 0x05 +#define HDMI_3D_TX_PHY_CPCE_CTRL 0x06 +#define HDMI_3D_TX_PHY_TXCLKMEASCTRL 0x07 +#define HDMI_3D_TX_PHY_TXMEASCTRL 0x08 +#define HDMI_3D_TX_PHY_CKSYMTXCTRL 0x09 +#define HDMI_3D_TX_PHY_CMPSEQCTRL 0x0a +#define
Re: [RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB
Hi Ville, Sorry for the late reply. On 11-01-2017 11:36, Ville Syrjälä wrote: > On Wed, Jan 11, 2017 at 10:27:03AM +, Jose Abreu wrote: >> Hi Ville, >> >> >> On 10-01-2017 17:21, Ville Syrjälä wrote: >> >> [snip] >> But we already have color_formats field in drm_display_info struct, right? Shouldn't we instead create for example a helper which returns the best output colorspace? According to what you said it would be something like: if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR444) return YCBCR444; else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR422) return YCBCR422; else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR420 && vic_is_420) return YCBCR420; else return RGB444; /* Mandatory by spec */ >>> Perhaps. But it would have to be more involved than that since there >>> might limitations on eg. the max TMDS clock imposed by the source or >>> cable/dongle which presumably might require that we pick 4:2:0 over >>> 4:4:4. >>> >>> It would also need to account which formats are actually supported by >>> the source. >>> >>> I guess for bpc it would be enough to just consider 8bpc in such a >>> function, and then the driver can bump it up afterwards if possible. >> But the max tmds clock will probably be involved in deep color >> modes, as 24bpp has always a 1x factor in every YCbCr, except >> 4:2:0. So, the sink has a max tmds but this gets into account >> when the vic list present in the EDID is built, but not >> considered in deep color modes, unless the EDID is broken. >> >>> As for the RGB vs. YCbCr question, I guess we should just prefer RGB444 >>> for now. And fall back to YCbCr 4:2:2 or 4:2:0 if necessary. And that >>> leaves YCbCr 4:4:4 unsed since it has the same requirements as RGB >>> 4:4:4 and thus doesn't provide any benefit as such. We could later add >>> a property to let the user choose between RGB vs. YCbCr more explicitly. >>> >> Hmm, I am trying to implement this but I am facing a difficulty: >> how will I fallback to YCbCr? RGB is always supported and the max >> tmds only enters in deep color modes. For reference here is a >> simple struct i created with the different tmds character rate >> factors for the different encodings (I think they are correct, >> but cross check please): >> >> #define DRM_CS_DESC(cs, f, b) \ >> .colorspace = (cs), .factor_to_khz = (f), .bpc = (b) >> >> static const struct drm_mode_colorspace_desc { >> u32 colorspace; >> u32 factor_to_khz; >> u32 bpc; >> } drm_mode_colorspace_factors = { /* Ordered by descending >> preference */ >> { DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 2000, 48) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 2000, 48) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 1500, 36) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 1500, 36) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 1250, 30) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 1250, 30) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 1000, 24) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 1000, 24) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB422, 1000, 24) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB422, 1000, 30) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB422, 1000, 36) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 1000, 48) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 750, 36) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 625, 30) }, >> { DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 500, 24) }, >> >> Notice how YCbCr 4:4:4 will never get picked: it has the same >> factor of RGB 4:4:4 for every bpc. So, the sink must support RGB >> 4:4:4 and may support YCbCr. What I didn't check was that if it >> is possible to have support for deep color YCbCr without having >> support for deep color RGB 4:4:4. Do you know if this can happen? > I don't think that's possible. So the 4:4:4 RGB vs. YCbCr choice is > probably something we have to leave up to the user. Although I have > a vague recollection that CEA-861 says that you should prefer YCbCr > for CE modes and RGB for IT modes. RGB Full Range is the default for IT modes. As for CE modes it says it depends on vactive, which I am not quite understanding why (pg. 34, CEA-861-F). > If we want to follow that I think we > want a property similar to the "Broadcast RGB" thing that allows you to > select between "Automatic", "RGB", and "YCbCr". Not sure if we should > also allow the user to explicitly select the subsampling mode for YCbCr. I think we can start by only RGB vs. YCbCr vs. automatic. > I also think we should probably still fall back to YCbCr 4:2:0 > automagically even if the user explicitly asked for RGB if we can't > light up the mode with RGB 4:4:4. > Agreed. I will start by that but in order for this to work in a real scenario (I got it working by changing EDID manually) the list of VIC's must be expanded to the new HDMI 2.0 VIC's. A patch was sent
Re: [PATCH 0/3] dw-hdmi: miscellaneous cleanups and fixes
Hi Laurent, Sorry for the late review. On 11-01-2017 23:49, Laurent Pinchart wrote: > Hello, > > These three small patches add to the 20 dw-hdmi patches previously submitted > in the "[PATCH v2 00/29] R-Car Gen3 HDMI output support" series. As only > patches 1 to 16 from that series have been approved and successfully tested > without any reported regression on all three dw-hdmi platforms, I've decided > to submit those three on top of the 16 first patches only and rebase the next > 4 after fixing them (which should happen soon). > > Patches 1 and 2 are cleanups and don't affect the generated code. Patch 3 > is a fix that moves SVSRET setting to the right location as needed by the > hardware (thank you Jose for reporting the problem). > > I plan to submit a pull request with the approved patches in the near future. > It will include these 3 patches if they can be acked soon enough. > > Laurent Pinchart (3): > drm: bridge: dw-hdmi: Define and use macros for PHY register addresses > drm: bridge: dw-hdmi: Fix the name of the PHY reset macros > drm: bridge: dw-hdmi: Assert SVSRET before resetting the PHY The whole patchset is: Reviewed-by: Jose AbreuThanks! :) Best regards, Jose Miguel Abreu > > drivers/gpu/drm/bridge/dw-hdmi.c | 45 ++ > drivers/gpu/drm/bridge/dw-hdmi.h | 69 > ++-- > 2 files changed, 92 insertions(+), 22 deletions(-) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 6/7] ARM: dts: stm32429i-eval: Enable ltdc & simple panel on Eval board
Hi On 01/16/2017 02:29 PM, Yannick Fertre wrote: Enable ltdc & enable am-480272h3tmqw-t01h panel. Signed-off-by: Yannick Fertre--- Can you please change commit header with: "ARM: dts: stm32: Enable ltdc & simple panel on stm32f429-Eval board" arch/arm/boot/dts/stm32429i-eval.dts | 58 1 file changed, 58 insertions(+) diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts index 2de6487..f987ca5 100644 --- a/arch/arm/boot/dts/stm32429i-eval.dts +++ b/arch/arm/boot/dts/stm32429i-eval.dts @@ -88,6 +88,52 @@ clocks = < 0 30>; clock-names = "main_clk"; }; + + panel_rgb: panel-rgb { + compatible = "ampire,am-480272h3tmqw-t01h"; + status = "okay"; + port { + panel_in_rgb: endpoint { + remote-endpoint = <_out_rgb>; + }; + }; + }; +}; + + { + pinctrl_ltdc: ltdc@0 { + pins { Pinmuxing definition is SOC specific. Please move it in stm32f429.dtsi file. Note that a development is ongoing to define pinmuxing in a dedicated pinctrl-stm32f4.dtsi file. + pinmux = , +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +, +; + slew-rate = <2>; + }; + }; }; _hse { @@ -123,3 +169,15 @@ pinctrl-names = "default"; status = "okay"; }; + +_host{ + status = "okay"; + pinctrl-0 = <_ltdc>; + pinctrl-names = "default"; + + port { + ltdc_out_rgb: endpoint { + remote-endpoint = <_in_rgb>; + }; + }; +}; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 7/7] ARM: configs: Add STM32 LTDC support in STM32 defconfig
Hi On 01/16/2017 02:29 PM, Yannick Fertre wrote: Signed-off-by: Yannick Fertre--- commit header "ARM: configs: stm32: ADD LDTC support" arch/arm/configs/stm32_defconfig | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 29068f5..e3974d9 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -71,3 +71,8 @@ CONFIG_MAGIC_SYSRQ=y # CONFIG_FTRACE is not set CONFIG_CRC_ITU_T=y CONFIG_CRC7=y +CONFIG_DRM=y +CONFIG_DRM_ST=y +CONFIG_DRM_PANEL=y +CONFIG_DRM_PANEL_SIMPLE=y +CONFIG_BACKLIGHT_LCD_SUPPORT=y ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
rockchip: edp: Meet a problem with supporting eDP panel at firefly release
Hello: I meet a problem when I want to add support for a lg,lp079qx1-sp0v eDP panel at firefly release rk3288 platform. I could hardly make the eDP work both on firefly release and firefly reload. Does you have any idea about that? [ 11.136586] i2c i2c-6: of_i2c: modalias failure on /dp@ff97/ports [ 11.143127] i2c i2c-6: Failed to create I2C device for /dp@ff97/ports [ 11.150288] rockchip-drm display-subsystem: bound ff97.dp (ops rockchip_dp_component_ops [analogix_dp_rockchip]) [ 11.160944] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 11.167645] [drm] No driver support for vblank timestamp query. [ OK ] Started LSB: Load kernel modules needed to enable cpufreq scaling. [ 11.205908] rockchip-drm display-subsystem: fb0: frame buffer device [ 11.221525] [drm] Initialized rockchip 1.0.0 20140818 for display-subsystem on minor 0 [ 11.275427] rockchip-dp ff97.dp: AUX CH command reply failed! [ OK ] Created slice system-systemd\x2dbacklight.slice. Starting Load/Save Screen Backlight...htness of backlight:backlight... Starting LSB: set CPUFreq kernel parameters... [ 11.450557] rockchip-dp ff97.dp: Rx Max Link Rate is abnormal :20 ! [ 11.457274] rockchip-dp ff97.dp: Rx Max Lane count is abnormal :0 ! [ 11.480023] rockchip-dp ff97.dp: AUX CH command reply failed! [ 11.532742] rockchip-dp ff97.dp: AUX CH command reply failed! [ 11.595626] rockchip-dp ff97.dp: LT link start failed! [ 11.601167] rockchip-dp ff97.dp: eDP link training failed (-121) [ OK ] Started Load/Save Screen Backlight Brightness of backlight:backlight. [ 11.622300] rockchip-dp ff97.dp: AUX CH command reply failed! [ 11.637980] rockchip-dp ff97.dp: AUX CH command reply failed! [ 11.667878] rockchip-dp ff97.dp: AUX CH command reply failed! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] clk: samsung: Remove Exynos4415 driver (SoC not supported anymore)
Hi, On 2017년 01월 14일 21:36, Krzysztof Kozlowski wrote: > Support for Exynos4415 is going away because there are no internal nor > external users. > > Since commit 46dcf0ff0de3 ("ARM: dts: exynos: Remove exynos4415.dtsi"), > the platform cannot be instantiated so remove also the drivers. > > Signed-off-by: Krzysztof Kozlowski> --- > .../devicetree/bindings/clock/exynos4415-clock.txt | 38 - > drivers/clk/samsung/Makefile |1 - > drivers/clk/samsung/clk-exynos4415.c | 1022 > > include/dt-bindings/clock/exynos4415.h | 360 --- > 4 files changed, 1421 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/clock/exynos4415-clock.txt > delete mode 100644 drivers/clk/samsung/clk-exynos4415.c > delete mode 100644 include/dt-bindings/clock/exynos4415.h Reviewed-by: Chanwoo Choi [snip] -- Best Regards, Chanwoo Choi S/W Center, Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Intel-gfx] [PATCH] drm: Fix for invalid pruning of modes in dual display cases
> -Original Message- > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] > Sent: Tuesday, December 13, 2016 9:17 PM > To: Jani Nikula> Cc: Chris Wilson ; Srinivas, Vidya > ; intel-...@lists.freedesktop.org; Syrjala, Ville > ; dri-devel@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm: Fix for invalid pruning of modes in dual > display cases > > On Tue, Dec 13, 2016 at 04:52:01PM +0200, Jani Nikula wrote: > > On Tue, 13 Dec 2016, Chris Wilson wrote: > > > On Tue, Dec 13, 2016 at 03:46:54PM +0530, Vidya Srinivas wrote: > > >> Currently in dual display connected boot scenarios, minimum of the > > >> resolutions is taken for fb width and height as reference. Based on > > >> this resolution, other modes are pruned. > > >> > > >> Example Scenario: If DSI mode is 2560x1440 and HDMI is 1920x1080, > > >> during the probing the fb width and height is set to max 1920x1080 > > >> and the DSI mode gets pruned as it is more than the reference. As a > result, there is no DSI display. > > >> Patch fixes this issue by taking the max of the resolutions and > > >> creating the fb based on the same. > > > > > > On the other hand, the viewable content is defined by the smaller mode. > > > If that is the only output visible at panic time, you don't want > > > that information lost due to it being invisible. > > > > > > This is only used for fbcon, which has to be the lowest common > > > denominator. Any actual application can set their own modes and fb. > > > > Do we fail to take over non-native modes with scaling on DSI then? > > Ville? > > I believe we are taking the panel fitter into account when we reconstruct the > BIOS fb. So in case the BIOS is upscaling we'll come up with some less than > screen size framebuffer. The fb helper will want to use the native mode of > the panel however, so at some point we'll drop the BIOS fb (as it's too small) > and allocate a new one. We are not sure how to take this further. Can you please provide some inputs for next steps on this? We will try to make the changes and test as per your inputs/feedback. > > -- > Ville Syrjälä > Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] pinctrl: samsung: Remove support for Exynos4415 (SoC not supported anymore)
On 2017년 01월 14일 21:36, Krzysztof Kozlowski wrote: > Support for Exynos4415 is going away because there are no internal nor > external users. > > Since commit 46dcf0ff0de3 ("ARM: dts: exynos: Remove exynos4415.dtsi"), > the platform cannot be instantiated so remove also the drivers. > > Signed-off-by: Krzysztof Kozlowski> --- > drivers/pinctrl/samsung/pinctrl-exynos.c | 75 > --- > drivers/pinctrl/samsung/pinctrl-samsung.c | 2 - > drivers/pinctrl/samsung/pinctrl-samsung.h | 1 - > 3 files changed, 78 deletions(-) Reviewed-by: Chanwoo Choi [snip] -- Best Regards, Chanwoo Choi S/W Center, Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 5/7] ARM: dts: stm32f429: Add ltdc support
Hi, On 01/16/2017 02:29 PM, Yannick Fertre wrote: Add LTDC (Lcd-tft Display Controller) support. Signed-off-by: Yannick FertreCan you change commit header by: "ARM: dts: stm32: Add ltdc support on stm32f429 MCU" please --- arch/arm/boot/dts/stm32f429.dtsi | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi index 336ee4f..fc43415 100644 --- a/arch/arm/boot/dts/stm32f429.dtsi +++ b/arch/arm/boot/dts/stm32f429.dtsi @@ -185,7 +185,7 @@ interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>; }; - pin-controller { + pinctrl: pin-controller { #address-cells = <1>; #size-cells = <1>; compatible = "st,stm32f429-pinctrl"; @@ -404,6 +404,29 @@ interrupts = <80>; clocks = < 0 38>; }; + + st-display-subsystem { + compatible = "st,display-subsystem"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + dma-ranges; + + ltdc_host: stm32-ltdc@40016800 { + compatible = "st,ltdc"; + reg = <0x40016800 0x200>; + interrupts = <88>, <89>; + resets = < 314>; + clocks = < 1 8>; + clock-names = "clk-lcd"; + status = "disabled"; + + port { + ltdc_out_rgb: endpoint { + }; + }; + }; + }; }; }; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Revert "drm/vmwgfx: Replace numeric parameter like 0444 with macro"
This reverts commit 2d8e60e8b0742b7a5cddc806fe38bb81ee876c33. The commit belongs to the series of 1285 patches sent to LKML on 2016-08-02. The general consensus was that the changes does not increase readability, quite the opposite; 0600 is easier to parse mentally than S_IRUSR | S_IWUSR. It also causes argument inconsistency, due to commit 04319d89fbec ("drm/vmwgfx: Add an option to change assumed FB bpp") that added another call to module_param_named() where the permissions are written as 0600. Signed-off-by: Øyvind A. Holm--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c index 18061a4bc2f2..e8ae3dc476d1 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c @@ -241,15 +241,15 @@ static int vmwgfx_pm_notifier(struct notifier_block *nb, unsigned long val, void *ptr); MODULE_PARM_DESC(enable_fbdev, "Enable vmwgfx fbdev"); -module_param_named(enable_fbdev, enable_fbdev, int, S_IRUSR | S_IWUSR); +module_param_named(enable_fbdev, enable_fbdev, int, 0600); MODULE_PARM_DESC(force_dma_api, "Force using the DMA API for TTM pages"); -module_param_named(force_dma_api, vmw_force_iommu, int, S_IRUSR | S_IWUSR); +module_param_named(force_dma_api, vmw_force_iommu, int, 0600); MODULE_PARM_DESC(restrict_iommu, "Try to limit IOMMU usage for TTM pages"); -module_param_named(restrict_iommu, vmw_restrict_iommu, int, S_IRUSR | S_IWUSR); +module_param_named(restrict_iommu, vmw_restrict_iommu, int, 0600); MODULE_PARM_DESC(force_coherent, "Force coherent TTM pages"); -module_param_named(force_coherent, vmw_force_coherent, int, S_IRUSR | S_IWUSR); +module_param_named(force_coherent, vmw_force_coherent, int, 0600); MODULE_PARM_DESC(restrict_dma_mask, "Restrict DMA mask to 44 bits with IOMMU"); -module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, S_IRUSR | S_IWUSR); +module_param_named(restrict_dma_mask, vmw_restrict_dma_mask, int, 0600); MODULE_PARM_DESC(assume_16bpp, "Assume 16-bpp when filtering modes"); module_param_named(assume_16bpp, vmw_assume_16bpp, int, 0600); -- 2.11.0.295.gd7dffce1cebd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] drm/atomic: Add accessor macros for all atomic state.
Hi Maarten, Thank you for the patches. On Monday 16 Jan 2017 10:37:37 Maarten Lankhorst wrote: > Fourth iteration. Instead of trying to convert all drivers straight away, > implement all macros that are required to get state working. > > Old situation: > Use obj->state, which can refer to old or new state. > Use drm_atomic_get_(existing_)obj_state, which can refer to new or old > state. Use for_each_obj_in_state, which refers to new or old state. > > New situation: > > During atomic check: > - Use drm_atomic_get_obj_state to add a object to the atomic state, > or get the new state. > - Use drm_atomic_get_(old/new)_obj_state to peek at the new/old state, > without adding the object. This will return NULL if the object is > not part of the state. For planes and connectors the relevant crtc_state > is added, so this will work to get the crtc_state from obj_state->crtc > too, this means not having to write some error handling. > > During atomic commit: > - Do not use drm_atomic_get_obj_state, obj->state or > drm_atomic_get_(existing_)obj_state any more, replace with > drm_atomic_get_old/new_obj_state calls as required. That will make driver code quite complicated :-/ I wonder if that's really a good solution. Maybe we should maintain obj->state only for the duration of the commit as a way to simplify drivers, and set it to NULL at all other times to avoid misusing it ? I know this contradicts the comment I've made in my review of patch 1/7, you can now choose which one to ignore :-) > During both: > - Use for_each_(new,old,oldnew)_obj_in_state to get the old or new state as > needed. oldnew will be renamed to for_each_obj_in_state after all callers > are converted to the new api. > > When not doing atomic updates: > Look at obj->state for now. I have some patches to fix this but I was asked > to make it return a const state. This breaks a lot of users though so I > skipped that patch in this iteration. > > This series will return the correct state regardless of swapping. > > Maarten Lankhorst (7): > drm/atomic: Add new iterators over all state, v3. > drm/atomic: Make add_affected_connectors look at crtc_state. > drm/atomic: Use new atomic iterator macros. > drm/atomic: Fix atomic helpers to use the new iterator macros. > drm/atomic: Add macros to access existing old/new state > drm/atomic: Convert get_existing_state callers to get_old/new_state, v2. > drm/blend: Use new atomic iterator macros. > > drivers/gpu/drm/drm_atomic.c | 39 ++-- > drivers/gpu/drm/drm_atomic_helper.c | 377 ++-- > drivers/gpu/drm/drm_blend.c | 23 +-- > drivers/gpu/drm/i915/intel_display.c | 13 +- > include/drm/drm_atomic.h | 180 - > include/drm/drm_atomic_helper.h | 2 + > 6 files changed, 438 insertions(+), 196 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/7] drm/atomic: Use new atomic iterator macros.
Hi Maarten, Thank you for the patch. On Monday 16 Jan 2017 10:37:40 Maarten Lankhorst wrote: Could we please get a description ? Apart from that, Reviewed-by: Laurent Pinchart> Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/drm_atomic.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 18cdf2c956c6..7b61e0da9ace 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state > *state) > > DRM_DEBUG_ATOMIC("checking %p\n", state); > > - for_each_plane_in_state(state, plane, plane_state, i) { > + for_each_new_plane_in_state(state, plane, plane_state, i) { > ret = drm_atomic_plane_check(plane, plane_state); > if (ret) { > DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n", > @@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state > *state) } > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > ret = drm_atomic_crtc_check(crtc, crtc_state); > if (ret) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check failed\n", > @@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state > *state) ret = config->funcs->atomic_check(state->dev, state); > > if (!state->allow_modeset) { > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > if (drm_atomic_crtc_needs_modeset(crtc_state)) { > DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full modeset\n", >crtc->base.id, crtc->name); > @@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct > drm_atomic_state *state) > > DRM_DEBUG_ATOMIC("checking %p\n", state); > > - for_each_plane_in_state(state, plane, plane_state, i) > + for_each_new_plane_in_state(state, plane, plane_state, i) > drm_atomic_plane_print_state(, plane_state); > > - for_each_crtc_in_state(state, crtc, crtc_state, i) > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) > drm_atomic_crtc_print_state(, crtc_state); > > - for_each_connector_in_state(state, connector, connector_state, i) > + for_each_new_connector_in_state(state, connector, connector_state, i) > drm_atomic_connector_print_state(, connector_state); > } > > @@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct drm_device > *dev, if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) > return 0; > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > u64 __user *fence_ptr; > > fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc); > @@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct drm_device > *dev, return; > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) { > /* >* TEST_ONLY and PAGE_FLIP_EVENT are mutually >* exclusive, if they weren't, this code should be -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch] drm/vc4: fix a bounds check
On Friday, 2017-01-13 10:49:00 +0300, Dan Carpenter wrote: > We accidentally return success even if vc4_full_res_bounds_check() fails. > > Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.") > Signed-off-by: Dan Carpenter> --- > Not tested. It would be good to test it, but the previous code would always return 0, and from a quick look the callers expect non-zero values on error, so this makes more sense at least. Reviewed-by: Eric Engestrom > > diff --git a/drivers/gpu/drm/vc4/vc4_render_cl.c > b/drivers/gpu/drm/vc4/vc4_render_cl.c > index 08886a3..5cdd003 100644 > --- a/drivers/gpu/drm/vc4/vc4_render_cl.c > +++ b/drivers/gpu/drm/vc4/vc4_render_cl.c > @@ -461,7 +461,7 @@ static int vc4_rcl_surface_setup(struct vc4_exec_info > *exec, > } > > ret = vc4_full_res_bounds_check(exec, *obj, surf); > - if (!ret) > + if (ret) > return ret; > > return 0; This now boils down to `return vc4_full_res_bounds_check(...);`, so you could get rid of the `ret` variable completely :) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] drm/bridge: adv7511: Re-write the i2c address as it may have been lost
On Mon, Jan 16, 2017 at 3:36 PM, Laurent Pinchartwrote: > Hi John, > > Thank you for the patch. > > On Monday 16 Jan 2017 15:16:51 John Stultz wrote: >> Laurent: Would something like the following be preferred? Seems >> to work as well for me.. > > That looks good to me. Feel free to still de-duplicate the power on/off code > if you want (but of course without adding the regcache_sync to the common > power on function this time). Ok. Will do that. Thanks again for the feedback/direction here! >> @@ -576,6 +578,9 @@ static int adv7511_get_modes(struct adv7511 *adv7511, >>ADV7511_INT1_DDC_ERROR); >> } >> adv7511->current_edid_segment = -1; >> + >> + /* Reset the EDID_I2C_ADDR register as it may have been > cleared */ >> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, > edid_i2c_addr); > > As powering the device off called regcache_mark_dirty(), this will perform an > I2C write and cache the value. If we then try to read the EDID a second time > without going through a full power on/off sequence, I believe that regmap will > skip the write the second time, as the cache will contain the same value and > won't be marked as dirty. Should we call regcache_mark_dirty() when powering > the device down further down this function ? Ah. Right. I had this in my earlier attempt, but thought I was simplifying things here. I'll correct this in the next revision. Thanks again! -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] drm/bridge: adv7511: Re-write the i2c address as it may have been lost
Hi John, Thank you for the patch. On Monday 16 Jan 2017 15:16:51 John Stultz wrote: > Laurent: Would something like the following be preferred? Seems > to work as well for me.. That looks good to me. Feel free to still de-duplicate the power on/off code if you want (but of course without adding the regcache_sync to the common power on function this time). Please see below for an additional comment. > I've found that by just turning the chip on and off via the > POWER_DOWN register, I end up getting i2c_transfer errors on > HiKey. > > Investigating further, it seems some of the register state in > the regmap cache is somehow getting lost, likely as the device > registers were reset during power off. > > Thus this patch simply re-writes the i2c address to the > ADV7511_REG_EDID_I2C_ADDR register to ensure its properly set > before we try to read the EDID data. > > Cc: David Airlie> Cc: Archit Taneja > Cc: Wolfram Sang > Cc: Lars-Peter Clausen > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 405e460..32c59cb > 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -567,6 +567,8 @@ static int adv7511_get_modes(struct adv7511 *adv7511, > > /* Reading the EDID only works if the device is powered */ > if (!adv7511->powered) { > + unsigned int edid_i2c_addr = (adv7511->i2c_main->addr << 1) + 4; > + > regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, > ADV7511_POWER_POWER_DOWN, 0); > if (adv7511->i2c_main->irq) { > @@ -576,6 +578,9 @@ static int adv7511_get_modes(struct adv7511 *adv7511, >ADV7511_INT1_DDC_ERROR); > } > adv7511->current_edid_segment = -1; > + > + /* Reset the EDID_I2C_ADDR register as it may have been cleared */ > + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); As powering the device off called regcache_mark_dirty(), this will perform an I2C write and cache the value. If we then try to read the EDID a second time without going through a full power on/off sequence, I believe that regmap will skip the write the second time, as the cache will contain the same value and won't be marked as dirty. Should we call regcache_mark_dirty() when powering the device down further down this function ? > } > > edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/atomic: Make add_affected_connectors look at crtc_state.
Hi Maarten, Thank you for the patch. On Monday 16 Jan 2017 10:37:39 Maarten Lankhorst wrote: > This kills another dereference of connector->state. connector_mask > holds all unchanged connectors at least and any changed connectors > are already in state anyway. > > Signed-off-by: Maarten LankhorstReviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/drm_atomic.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 1c1cbf436717..18cdf2c956c6 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1419,8 +1419,13 @@ drm_atomic_add_affected_connectors(struct > drm_atomic_state *state, struct drm_connector *connector; > struct drm_connector_state *conn_state; > struct drm_connector_list_iter conn_iter; > + struct drm_crtc_state *crtc_state; > int ret; > > + crtc_state = drm_atomic_get_crtc_state(state, crtc); > + if (IS_ERR(crtc_state)) > + return PTR_ERR(crtc_state); > + > ret = drm_modeset_lock(>connection_mutex, state->acquire_ctx); > if (ret) > return ret; > @@ -1429,12 +1434,12 @@ drm_atomic_add_affected_connectors(struct > drm_atomic_state *state, crtc->base.id, crtc->name, state); > > /* > - * Changed connectors are already in @state, so only need to look at the > - * current configuration. > + * Changed connectors are already in @state, so only need to look > + * at the connector_mask in crtc_state. >*/ > drm_connector_list_iter_get(state->dev, _iter); > drm_for_each_connector_iter(connector, _iter) { > - if (connector->state->crtc != crtc) > + if (!(crtc_state->connector_mask & (1 << > drm_connector_index(connector continue; > > conn_state = drm_atomic_get_connector_state(state, connector); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC][PATCH] drm/bridge: adv7511: Re-write the i2c address as it may have been lost
Laurent: Would something like the following be preferred? Seems to work as well for me.. thanks -john I've found that by just turning the chip on and off via the POWER_DOWN register, I end up getting i2c_transfer errors on HiKey. Investigating further, it seems some of the register state in the regmap cache is somehow getting lost, likely as the device registers were reset during power off. Thus this patch simply re-writes the i2c address to the ADV7511_REG_EDID_I2C_ADDR register to ensure its properly set before we try to read the EDID data. Cc: David AirlieCc: Archit Taneja Cc: Wolfram Sang Cc: Lars-Peter Clausen Cc: Laurent Pinchart Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 405e460..32c59cb 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -567,6 +567,8 @@ static int adv7511_get_modes(struct adv7511 *adv7511, /* Reading the EDID only works if the device is powered */ if (!adv7511->powered) { + unsigned int edid_i2c_addr = (adv7511->i2c_main->addr << 1) + 4; + regmap_update_bits(adv7511->regmap, ADV7511_REG_POWER, ADV7511_POWER_POWER_DOWN, 0); if (adv7511->i2c_main->irq) { @@ -576,6 +578,9 @@ static int adv7511_get_modes(struct adv7511 *adv7511, ADV7511_INT1_DDC_ERROR); } adv7511->current_edid_segment = -1; + + /* Reset the EDID_I2C_ADDR register as it may have been cleared */ + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); } edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/7] drm/atomic: Add new iterators over all state, v3.
Hi Maarten, Thank you for the patch. On Monday 16 Jan 2017 10:37:38 Maarten Lankhorst wrote: > Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to > replace the old for_each_xxx_in_state ones. This is useful for >1 flip > depth and getting rid of all xxx->state dereferences. > > This requires extra fixups done when committing a state after > duplicating, which in general isn't valid but is used by suspend/resume. > To handle these, introduce drm_atomic_helper_commit_duplicated_state > which performs those fixups before checking & committing the state. > > Changes since v1: > - Remove nonblock parameter for commit_duplicated_state. > Changes since v2: > - Use commit_duplicated_state for i915 load detection. > - Add WARN_ON(old_state != obj->state) before swapping. > > Signed-off-by: Maarten Lankhorst> --- > drivers/gpu/drm/drm_atomic.c | 6 +++ > drivers/gpu/drm/drm_atomic_helper.c | 65 + > drivers/gpu/drm/i915/intel_display.c | 13 +++--- > include/drm/drm_atomic.h | 81 +++-- > include/drm/drm_atomic_helper.h | 2 + > 5 files changed, 149 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 6414bcf7f41b..1c1cbf436717 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -275,6 +275,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state > *state, return ERR_PTR(-ENOMEM); > > state->crtcs[index].state = crtc_state; > + state->crtcs[index].old_state = crtc->state; > + state->crtcs[index].new_state = crtc_state; > state->crtcs[index].ptr = crtc; > crtc_state->state = state; > > @@ -691,6 +693,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state > *state, > > state->planes[index].state = plane_state; > state->planes[index].ptr = plane; > + state->planes[index].old_state = plane->state; > + state->planes[index].new_state = plane_state; > plane_state->state = state; > > DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n", > @@ -1031,6 +1035,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state > *state, > > drm_connector_reference(connector); > state->connectors[index].state = connector_state; > + state->connectors[index].old_state = connector->state; > + state->connectors[index].new_state = connector_state; > state->connectors[index].ptr = connector; > connector_state->state = state; > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index b26e3419027e..d153e8a72921 > 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1971,11 +1971,11 @@ void drm_atomic_helper_swap_state(struct > drm_atomic_state *state, int i; > long ret; > struct drm_connector *connector; > - struct drm_connector_state *conn_state; > + struct drm_connector_state *conn_state, *old_conn_state; > struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *crtc_state, *old_crtc_state; > struct drm_plane *plane; > - struct drm_plane_state *plane_state; > + struct drm_plane_state *plane_state, *old_plane_state; > struct drm_crtc_commit *commit; > > if (stall) { > @@ -1999,13 +1999,17 @@ void drm_atomic_helper_swap_state(struct > drm_atomic_state *state, } > } > > - for_each_connector_in_state(state, connector, conn_state, i) { > + for_each_oldnew_connector_in_state(state, connector, old_conn_state, > conn_state, i) { > + WARN_ON(connector->state != old_conn_state); > + > connector->state->state = state; > swap(state->connectors[i].state, connector->state); > connector->state->state = NULL; > } > > - for_each_crtc_in_state(state, crtc, crtc_state, i) { > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, > i) { > + WARN_ON(crtc->state != old_crtc_state); > + > crtc->state->state = state; > swap(state->crtcs[i].state, crtc->state); > crtc->state->state = NULL; > @@ -2020,7 +2024,9 @@ void drm_atomic_helper_swap_state(struct > drm_atomic_state *state, } > } > > - for_each_plane_in_state(state, plane, plane_state, i) { > + for_each_oldnew_plane_in_state(state, plane, old_plane_state, > plane_state, i) { > + WARN_ON(plane->state != old_plane_state); > + > plane->state->state = state; > swap(state->planes[i].state, plane->state); > plane->state->state = NULL; > @@ -2471,7 +2477,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all); > * > * See also: > * drm_atomic_helper_duplicate_state(), drm_atomic_helper_disable_all(), > - * drm_atomic_helper_resume() > + * drm_atomic_helper_resume(),
Re: Unix Device Memory Allocation project
Thanks for all the feedback. Things are much clearer now. Yeah, we can use the BO modifiers for simple 2D images / planes if that's the general direction. I think we can even stuff the compression data buffer offset into those 64 bits, considering it's not very large (e.g. below 4GB and low bits are unused due to alignment). For OpenCL at least, we have to keep using the 256-bytes-large per-BO metadata to describe more complex allocations. Marek On Wed, Jan 4, 2017 at 5:59 PM, Daniel Stonewrote: > Hi Christian, > > On 4 January 2017 at 16:02, Christian König wrote: >> Am 04.01.2017 um 16:47 schrieb Rob Clark: >>> If the position of the different parts of the buffer are somewhere >>> required to be a function of w/h/bpp/etc then I'm not sure if there is >>> a strong advantage to treating them as separate BOs.. although I >>> suppose it doesn't preclude it either. As far as plumbing it through >>> mesa/st, it seems convenient to have a single buffer. (We have kind >>> of a hack to deal w/ multi-planar yuv, but I'd rather not propagate >>> that.. but I've not thought through those details so much yet.) >> >> Well I don't want to ruin your day, but there are different requirements >> from different hardware. >> >> For example the UVD engine found in all AMD graphics cards since r600 must >> have both planes in a single BO because the memory controller can only >> handle a rather small offset between the planes. > > This is, to a large extent, also true of Intel. > >> On the other hand I know of embedded MPEG2/H264 decoders where the different >> planes must be on different memory channels. In this case I can imagine >> you want one BO for each plane, because otherwise the device must stitch >> together one buffer object from two different memory regions (of course >> possible, but rather ugly). > > Not just embedded, but quite a few platforms where the ratio of > required to available memory bandwidth is ... somewhat different to > larger discrete systems. Striping allocations such that luma and > chroma live on different memory channels isn't uncommon. > > But I think this is all orthogonal. If you keep auxiliary planes in > separate BOs to metadata, you can still handle both cases. How to > place buffers is purely an _allocation_ concern, where single vs. > multiple BO is purely about addressing them. So your allocator API may > become a little more complex - something which only device-specific > userspace will ever address - whilst keeping a unified > addressing/handle system for the generic parts of userspace which > shouldn't have to care about whether the underlying hardware demands a > small offset or a completely separate allocation. > > Having API pegged to the single-underlying-BO concept has been a giant > pain for those who can't use single BOs. I don't see anything good > coming of the idea for cross-device/cross-vendor sharing either, since > it encodes yet more magic implicit detail into buffer sharing. Since > that detail ultimately has to be resolved _somewhere_, it's a problem > avoided rather than a problem solved. > > Cheers, > Daniel > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5 v3] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID
Hi John, On Monday 16 Jan 2017 12:14:48 John Stultz wrote: > On Mon, Jan 16, 2017 at 8:03 AM, Laurent Pinchart wrote: > > On Tuesday 03 Jan 2017 11:41:42 John Stultz wrote: > >> I've found that by just turning the chip on and off via the > >> POWER_DOWN register, I end up getting i2c_transfer errors > >> on HiKey. > >> > >> Investigating further, it seems some of the register state > >> in the regmap cache is somehow getting lost. Using the logic > >> in __adv7511_power_on/off() which syncs and dirtys the cache > >> avoids this issue. > >> > >> Thus this patch changes the EDID probing logic so that we > >> re-use the __adv7511_power_on/off() calls. > > > > regcache_sync() is quite costly as it will write a bunch of registers. > > Wouldn't it be more efficient to only write the registers that are needed > > for EDID access ? > > So yes, you've mentioned this concern before, and I did spend some > time to narrow which lost-register state (0x43 > - ADV7511_REG_EDID_I2C_ADDR) was causing the trouble with i2c > trasnfer errors I was seeing: > https://lkml.org/lkml/2016/11/22/677 > > However, I didn't get much feedback on that, and it seems (to me at > least) concerning that we are losing the underlying state of a > register in the cache, so just syncing that one register back to the > hardware might solve the issue I was seeing, but I worry what other > registers might also be out of sync. > > The comment above the regmap_sync in adv7511_power_on after all states: >"Most of the registers are reset during power down or when HPD is low." You're right that most registers will be out of sync. > So it seems like if we're setting the power down (and setting HPD in > cases where Archit had a patch to add HPD pulsing to the > adv7511_get_modes path), it seems reasonable to do the same > regmap_sync()? It would be if we had to keep the device powered up, but we're powering it down right after reading the EDID. I don't think there's a need to reconfigure it completely, only setting the registers needed to read the EDID should be enough. > But, I'm not really picky here, and I'm very open to other approaches > (including something like the patch in the link above) if you have > suggestions/preferences. I just want it to work reliably on my > hardware. :) > > And just so I can better understand it, can you explain some about the > impact of your efficiency concerns? I'm not too picky either :-) If we can't find a reliable way to read the EDID by just configuring the registers we need, we could go for a full reconfiguration. However, restoring the value of all cached registers will result in lots of I2C writes, which are time-consuming operations. EDID read would be sped up if we could avoid that. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fence: fix memory overwrite when setting out_fence fd
2017-01-13 Chad Versace: > On Fri 13 Jan 2017, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Currently if the userspace declares a int variable to store the out_fence > > fd and pass it to OUT_FENCE_PTR the kernel will overwrite the 32 bits > > above the int variable on 64 bits systems. > > > > Fix this by making the internal storage of out_fence in the kernel a s32 > > pointer. > > > > Reported-by: Chad Versace > > Signed-off-by: Gustavo Padovan > > Cc: Daniel Vetter > > Cc: Rafael Antognolli > > Cc: Laurent Pinchart > > Cc: sta...@vger.kernel.org > > Reviewed-and-Tested-by: Chad Versace > > I applied this to my kernel branch, updated kmscube, and the spinning cube > still looks good. > For reference, here are the tags I tested with: > > mesa: > http://git.kiwitree.net/cgit/~chadv/mesa/tag/?h=chadv/review/i965-exec-fence-v03 > libdrm: > http://git.kiwitree.net/cgit/~chadv/libdrm/tag/?h=chadv/review/intel-exec-fence-v01 > linux: > http://git.kiwitree.net/cgit/~chadv/linux/tag/?h=chadv/test/i915-exec-fence-v04 > kmscube: > http://git.kiwitree.net/cgit/~chadv/kmscube/tag/?h=chadv/test/fences-v03 I pushed this patch to drm-misc-fixes. Thank you all. Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state
Hi Andrey, Thank you for the patch. On Monday 16 Jan 2017 10:44:55 Andrey Grodzovsky wrote: > Allows using atomic flip helpers for drivers > using ASYNC flip. > Remove ASYNC_FLIP restriction in helpers and > caches the page flip flags in drm_plane_state > to be used in the low level drivers. > > Signed-off-by: Andrey Grodzovsky> --- > drivers/gpu/drm/drm_atomic_helper.c | 10 +++--- > include/drm/drm_plane.h | 8 > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..f83dc43 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2737,7 +2737,8 @@ static int page_flip_common( > struct drm_atomic_state *state, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event) > + struct drm_pending_vblank_event *event, > + uint32_t flags) > { > struct drm_plane *plane = crtc->primary; > struct drm_plane_state *plane_state; > @@ -2754,6 +2755,7 @@ static int page_flip_common( > if (IS_ERR(plane_state)) > return PTR_ERR(plane_state); > > + plane_state->pflip_flags = flags; > > ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > if (ret != 0) > @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc, > struct drm_atomic_state *state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - With this change all drivers using the helper will not reject that async flag, even if they don't implement support for async page flip. You need to either patch them all to reject the flag, or implement async page flip support for all of them (preferable in the helpers, and then remove the * Note that for now so called async page flips (i.e. updates which are not * synchronized to vblank) are not supported, since the atomic interfaces have * no provisions for this yet. comment). > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target( > struct drm_crtc_state *crtc_state; > int ret = 0; > > - if (flags & DRM_MODE_PAGE_FLIP_ASYNC) > - return -EINVAL; > - > state = drm_atomic_state_alloc(plane->dev); > if (!state) > return -ENOMEM; > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index db3bbde..86d8ffc 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -122,6 +122,14 @@ struct drm_plane_state { >*/ > bool visible; > > + > + /** > + * @pflip_flags: > + * > + * Flip related config options > + */ > + u32 pflip_flags; > + > struct drm_atomic_state *state; > }; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/amd/display: Switch to using atomic_helper for flip.
Hi Andrey, Thank you for the patch. On Monday 16 Jan 2017 10:44:57 Andrey Grodzovsky wrote: > Change-Id: Iad3e0b9b3546e4e4dc79be9233daf4fe4dba83e0 > Signed-off-by: Andrey Grodzovsky> --- > .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 ++ > 1 file changed, 6 insertions(+), 86 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c index > a443b70..d4664bf 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c > @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property( > return 0; > } > > - > -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc, > - struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event, > - uint32_t flags) > -{ > - struct drm_plane *plane = crtc->primary; > - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > - struct drm_atomic_state *state; > - struct drm_plane_state *plane_state; > - struct drm_crtc_state *crtc_state; > - int ret = 0; > - > - state = drm_atomic_state_alloc(plane->dev); > - if (!state) > - return -ENOMEM; > - > - ret = drm_crtc_vblank_get(crtc); The DRM core's atomic page flip helper doesn't get/put vblank. Have you double-checked that removing them isn't a problem ? > - if (ret) > - return ret; > - > - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); > -retry: > - crtc_state = drm_atomic_get_crtc_state(state, crtc); > - if (IS_ERR(crtc_state)) { > - ret = PTR_ERR(crtc_state); > - goto fail; > - } > - crtc_state->event = event; > - > - plane_state = drm_atomic_get_plane_state(state, plane); > - if (IS_ERR(plane_state)) { > - ret = PTR_ERR(plane_state); > - goto fail; > - } > - > - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); > - if (ret != 0) > - goto fail; > - drm_atomic_set_fb_for_plane(plane_state, fb); > - > - /* Make sure we don't accidentally do a full modeset. */ > - state->allow_modeset = false; > - if (!crtc_state->active) { > - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", > - crtc->base.id); > - ret = -EINVAL; > - goto fail; > - } > - acrtc->flip_flags = flags; > - > - ret = drm_atomic_nonblocking_commit(state); > - > -fail: > - if (ret == -EDEADLK) > - goto backoff; > - > - if (ret) > - drm_crtc_vblank_put(crtc); > - > - drm_atomic_state_put(state); > - > - return ret; > -backoff: > - drm_atomic_state_clear(state); > - drm_atomic_legacy_backoff(state); > - > - /* > - * Someone might have exchanged the framebuffer while we dropped locks > - * in the backoff code. We need to fix up the fb refcount tracking the > - * core does for us. > - */ > - plane->old_fb = plane->fb; > - > - goto retry; > -} > - > /* Implemented only the options currently availible for the driver */ > static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = { > .reset = drm_atomic_helper_crtc_reset, > @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct > drm_crtc *crtc, .destroy = amdgpu_dm_crtc_destroy, > .gamma_set = amdgpu_dm_atomic_crtc_gamma_set, > .set_config = drm_atomic_helper_set_config, > - .page_flip = amdgpu_atomic_helper_page_flip, > + .page_flip_target = drm_atomic_helper_page_flip_target, > .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > .atomic_set_property = dm_crtc_funcs_atomic_set_property > @@ -1679,7 +1602,7 @@ static bool page_flip_needed( > sizeof(old_state_tmp)) == 0 ? true:false; > if (new_state->crtc && page_flip_required == false) { > acrtc_new = to_amdgpu_crtc(new_state->crtc); > - if (acrtc_new->flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) > + if (new_state->pflip_flags & DRM_MODE_PAGE_FLIP_ASYNC) > page_flip_required = true; > } > return page_flip_required; > @@ -2760,7 +2683,6 @@ int amdgpu_dm_atomic_commit( > for_each_plane_in_state(state, plane, old_plane_state, i) { > struct drm_plane_state *plane_state = plane->state; > struct drm_crtc *crtc = plane_state->crtc; > - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); > struct drm_framebuffer *fb = plane_state->fb; > > if (!fb || !crtc || !crtc->state->planes_changed || > @@ -2771,10 +2693,9 @@ int amdgpu_dm_atomic_commit( > ret =
Re: [PATCH 2/4] drm/amdgpu: Remove flip_flag from amdgpu_crtc
Hi Andrey, Thank you for the patch. On Monday 16 Jan 2017 10:44:56 Andrey Grodzovsky wrote: > Follwing introduction of pflip_flags in drm_plane_state > this is not needed anymore. > > Signed-off-by: Andrey Grodzovsky> --- > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 4c0a86e..3ff3c14 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h > @@ -443,7 +443,6 @@ struct amdgpu_crtc { > enum amdgpu_interrupt_state vsync_timer_enabled; > > int otg_inst; > - uint32_t flip_flags; This breaks compilation of the amdgpu driver. You should squash this patch with 3/4. > /* After Set Mode target will be non-NULL */ > struct dc_target *target; > }; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers.
Hi Harry, On Monday 16 Jan 2017 16:13:39 Harry Wentland wrote: > On 2017-01-16 03:39 PM, Laurent Pinchart wrote: > > On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: > >> This series is a folow-up on > >> https://patchwork.kernel.org/patch/9501787/ > >> > >> The first patch makes changes to atomic helpers > >> to allow for drives with ASYNC flip support to use them. > >> Patches 2 and 3 are to use this in AMDGPU/DC and > >> patch 4 is possible cleanup in nouveau/kms who seems > >> to have the duplicate the helper as we did to support > >> ASYNC flips. > > > > I have my doubts regarding this. I'd much rather see userspace moving to > > the atomic API instead of extending support for legacy APIs. > > This change is not about introducing the async flag but cleaning up the > legacy helpers to make sure drivers that currently use it through the > legacy IOCTLs can benefit from the helpers and not have to roll their own. > > If the problem is with the pflip_flags, wouldn't drivers still need that > after moving userspace to the atomic IOCTL? > > I don't disagree with you on having userspace move to atomic but I don't > expect to see all userspace drivers move to atomic in the next couple > months. Why not clean this up in the meantime? If this patch series was just about moving common driver code into the core, sure, but it goes beyond that. Or, actually, it needs to go beyond that, but doesn't yet. Removing the DRM_MODE_PAGE_FLIP_ASYNC test in patch 1/4 means that the DRM core will not reject async page flips anymore, for any driver that uses the helper. You thus need to either patch all drivers that use the helper to reject the flag, or implement the feature in the drivers (and preferably in the helpers then). The current version of this patch series will make all existing users of the helpers accept async page flips without actually implementing them. > >> Andrey Grodzovsky (4): > >> drm/atomic: Save flip flags in drm_plane_state > >> drm/amdgpu: Remove flip_flag from amdgpu_crtc > >> drm/amd/display: Switch to using atomic_helper for flip. > >> drm/nouveau/kms/nv50: Switch to using atomic helper for flip. > >> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - > >> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 --- > >> drivers/gpu/drm/drm_atomic_helper.c| 10 +-- > >> drivers/gpu/drm/nouveau/nv50_display.c | 77 ++- > >> include/drm/drm_plane.h| 8 ++ > >> 5 files changed, 22 insertions(+), 166 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/armada: Include current dir on CFLAGS for armada trace
Hi Gustavo, (CC'ing Steven) On Monday 16 Jan 2017 19:12:58 Gustavo Padovan wrote: > 2017-01-16 Laurent Pinchart: > > On Monday 16 Jan 2017 18:13:30 Gustavo Padovan wrote: > >> From: Gustavo Padovan > >> > >> Otherwise compilation fails like this: > >> > >> In file included from drivers/gpu/drm/armada/armada_trace.h:66:0, > >> > >> from drivers/gpu/drm/armada/armada_trace.c:3: > >> ./include/trace/define_trace.h:88:43: fatal error: ./armada_trace.h: No > >> such file or directory compilation terminated. > >> > >> Signed-off-by: Gustavo Padovan > > > > How about a Fixes: line ? > > Fixes: c8a220c686a5 ("drm/armada: add tracing support") Thank you. The approach taken here seems to be shared by a fair number of drivers, so Acked-by: Laurent Pinchart However, you could also set TRACE_INCLUDE_PATH to ../../drivers/gpu/drm/armada as done by drivers/dma-buf, drivers/ras and drivers/net/fjes. I'm not sure what's best, but if setting CFLAGS is preferred, I think we should get rid of TRACE_INCLUDE_PATH. Steven, any opinion ? To avoid forcing you to dig the original e-mail up, the proposed fix is > diff --git a/drivers/gpu/drm/armada/Makefile > b/drivers/gpu/drm/armada/Makefile > index a18f156..64c0b45 100644 > --- a/drivers/gpu/drm/armada/Makefile > +++ b/drivers/gpu/drm/armada/Makefile > @@ -4,3 +4,5 @@ armada-y+= armada_510.o > armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o > > obj-$(CONFIG_DRM_ARMADA) := armada.o > + > +CFLAGS_armada_trace.o := -I$(src) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/atomic: make release_crtc_commit() static
Hi Wei, 2017-01-12 Wei Yongjun: > From: Wei Yongjun > > Fixes the following sparse warning: > > drivers/gpu/drm/drm_atomic_helper.c:1360:6: warning: > symbol 'release_crtc_commit' was not declared. Should it be static? > > Signed-off-by: Wei Yongjun > --- > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Gustavo Padovan Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 96488] [r600g]OpenCL driver causes segfault in ImageMagick's Histogram kernel
https://bugs.freedesktop.org/show_bug.cgi?id=96488 --- Comment #10 from Jan Vesely--- (In reply to nixscripter from comment #9) > I have downloaded LLVM r290690 which contains your first diff, manually > applied your 2nd diff, and compiled it. The resulting library seems to > resolve the issue. > > Once that patch is reviewed and applied to the mainline, I will do one final > verification and (hopefully) close this bug. > > Thanks for all your efforts on this! I have just landed the patch. Note that if the kernel uses sub-int(short,char) vectors. it will generate incorrect code until D27964 (or alternative) is merged. Jan -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/hisilicon/hibmc: Fix wrong pointer passed to PTR_ERR()
Hi Wei, 2017-01-12 Wei Yongjun: > From: Wei Yongjun > > PTR_ERR should access the value just tested by IS_ERR, otherwise > the wrong error code will be returned. > > Fixes: d1667b86795a ("drm/hisilicon/hibmc: Add support for frame buffer") > Signed-off-by: Wei Yongjun Reviewed-by: Gustavo Padovan Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers.
On 2017-01-16 03:39 PM, Laurent Pinchart wrote: Hi Andrey, Thank you for the patches. On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: This series is a folow-up on https://patchwork.kernel.org/patch/9501787/ The first patch makes changes to atomic helpers to allow for drives with ASYNC flip support to use them. Patches 2 and 3 are to use this in AMDGPU/DC and patch 4 is possible cleanup in nouveau/kms who seems to have the duplicate the helper as we did to support ASYNC flips. I have my doubts regarding this. I'd much rather see userspace moving to the atomic API instead of extending support for legacy APIs. This change is not about introducing the async flag but cleaning up the legacy helpers to make sure drivers that currently use it through the legacy IOCTLs can benefit from the helpers and not have to roll their own. If the problem is with the pflip_flags, wouldn't drivers still need that after moving userspace to the atomic IOCTL? I don't disagree with you on having userspace move to atomic but I don't expect to see all userspace drivers move to atomic in the next couple months. Why not clean this up in the meantime? Harry Andrey Grodzovsky (4): drm/atomic: Save flip flags in drm_plane_state drm/amdgpu: Remove flip_flag from amdgpu_crtc drm/amd/display: Switch to using atomic_helper for flip. drm/nouveau/kms/nv50: Switch to using atomic helper for flip. drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 --- drivers/gpu/drm/drm_atomic_helper.c| 10 +-- drivers/gpu/drm/nouveau/nv50_display.c | 77 ++ include/drm/drm_plane.h| 8 ++ 5 files changed, 22 insertions(+), 166 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm: qxl: Open code probing sequence for qxl
Hi Gabriel, 2017-01-12 Gabriel Krisman Bertazi: > This avoids using the deprecated drm_get_pci_dev() and load() hook > interfaces in the qxl driver. > > The only tricky part is to ensure TTM debugfs initialization > happens after the debugfs root node is created, which is done by moving > that code into the debufs_init() hook. Since the hook is called 3 times > for each minor function, we make sure it is only executed for the > primary minor. > > Tested on qemu with igt and running a WM on top of X. > > Signed-off-by: Gabriel Krisman Bertazi > CC: Dave Airlie > CC: Daniel Vetter > CC: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/qxl/qxl_debugfs.c | 12 > drivers/gpu/drm/qxl/qxl_drv.c | 58 > +-- > drivers/gpu/drm/qxl/qxl_drv.h | 7 - > drivers/gpu/drm/qxl/qxl_kms.c | 40 ++- > drivers/gpu/drm/qxl/qxl_ttm.c | 8 +- > 5 files changed, 77 insertions(+), 48 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c > b/drivers/gpu/drm/qxl/qxl_debugfs.c > index 241af9131dc8..417b538b3ed8 100644 > --- a/drivers/gpu/drm/qxl/qxl_debugfs.c > +++ b/drivers/gpu/drm/qxl/qxl_debugfs.c > @@ -84,8 +84,20 @@ int > qxl_debugfs_init(struct drm_minor *minor) > { > #if defined(CONFIG_DEBUG_FS) > + int r; > + struct qxl_device *dev = > + (struct qxl_device *) minor->dev->dev_private; > + > drm_debugfs_create_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES, >minor->debugfs_root, minor); > + > + if (minor->type == DRM_MINOR_PRIMARY) { I'm not confortable with exposing minor->type here and making qxl the first driver to use it outside of drm core. Don't we have any other way. I see that inside qxl_debugfs_add_files() we have a check for already registered files. Wouldn't that or some modification around work for this? Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/armada: Include current dir on CFLAGS for armada trace
2017-01-16 Laurent Pinchart: > Hi Gustavo, > > Thank you for the patch. > > On Monday 16 Jan 2017 18:13:30 Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Otherwise compilation fails like this: > > > > In file included from drivers/gpu/drm/armada/armada_trace.h:66:0, > > from drivers/gpu/drm/armada/armada_trace.c:3: > > ./include/trace/define_trace.h:88:43: fatal error: ./armada_trace.h: No such > > file or directory compilation terminated. > > > > Signed-off-by: Gustavo Padovan > > How about a Fixes: line ? Fixes: c8a220c686a5 ("drm/armada: add tracing support") Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99275] Kernel 4.9: amdgpu regression; gui flickers; amd radeon rx 460
https://bugs.freedesktop.org/show_bug.cgi?id=99275 --- Comment #10 from Reimar Imhof--- I've just tried kernel 4.9.4 from download.opensuse.org/repositories/Kernel:/stable/standard/ Same bug. Do you have any news? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state
> -Original Message- > From: Gustavo Padovan [mailto:gust...@padovan.org] > Sent: Monday, January 16, 2017 3:22 PM > To: Grodzovsky, Andrey > Cc: dri-devel@lists.freedesktop.org; nouv...@lists.freedesktop.org; amd- > g...@lists.freedesktop.org; Deucher, Alexander; daniel.vet...@intel.com > Subject: Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state > > Hi Andrey, > > 2017-01-16 Andrey Grodzovsky: > > > Allows using atomic flip helpers for drivers using ASYNC flip. > > Remove ASYNC_FLIP restriction in helpers and caches the page flip > > flags in drm_plane_state to be used in the low level drivers. > > > > Signed-off-by: Andrey Grodzovsky > > --- > > drivers/gpu/drm/drm_atomic_helper.c | 10 +++--- > > include/drm/drm_plane.h | 8 > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index a4e5477..f83dc43 100644 > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > @@ -2737,7 +2737,8 @@ static int page_flip_common( > > struct drm_atomic_state *state, > > struct drm_crtc *crtc, > > struct drm_framebuffer *fb, > > - struct drm_pending_vblank_event *event) > > + struct drm_pending_vblank_event *event, > > + uint32_t flags) > > Did you build this patch? It is changing the signature of > page_flip_common() but no changes to the callers. > > Gustavo Thanks for spotting this, I am afraid I've sent not the final version of the patch. I will resend the latest version later today. Thanks Andrey ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] Allow ASYNC flip with atomic helpers.
Hi Andrey, Thank you for the patches. On Monday 16 Jan 2017 10:44:54 Andrey Grodzovsky wrote: > This series is a folow-up on > https://patchwork.kernel.org/patch/9501787/ > > The first patch makes changes to atomic helpers > to allow for drives with ASYNC flip support to use them. > Patches 2 and 3 are to use this in AMDGPU/DC and > patch 4 is possible cleanup in nouveau/kms who seems > to have the duplicate the helper as we did to support > ASYNC flips. I have my doubts regarding this. I'd much rather see userspace moving to the atomic API instead of extending support for legacy APIs. > Andrey Grodzovsky (4): > drm/atomic: Save flip flags in drm_plane_state > drm/amdgpu: Remove flip_flag from amdgpu_crtc > drm/amd/display: Switch to using atomic_helper for flip. > drm/nouveau/kms/nv50: Switch to using atomic helper for flip. > > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 - > .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 92 --- > drivers/gpu/drm/drm_atomic_helper.c| 10 +-- > drivers/gpu/drm/nouveau/nv50_display.c | 77 ++ > include/drm/drm_plane.h| 8 ++ > 5 files changed, 22 insertions(+), 166 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/armada: Include current dir on CFLAGS for armada trace
Hi Gustavo, Thank you for the patch. On Monday 16 Jan 2017 18:13:30 Gustavo Padovan wrote: > From: Gustavo Padovan> > Otherwise compilation fails like this: > > In file included from drivers/gpu/drm/armada/armada_trace.h:66:0, > from drivers/gpu/drm/armada/armada_trace.c:3: > ./include/trace/define_trace.h:88:43: fatal error: ./armada_trace.h: No such > file or directory compilation terminated. > > Signed-off-by: Gustavo Padovan How about a Fixes: line ? > --- > drivers/gpu/drm/armada/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/armada/Makefile > b/drivers/gpu/drm/armada/Makefile index a18f156..64c0b45 100644 > --- a/drivers/gpu/drm/armada/Makefile > +++ b/drivers/gpu/drm/armada/Makefile > @@ -4,3 +4,5 @@ armada-y += armada_510.o > armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o > > obj-$(CONFIG_DRM_ARMADA) := armada.o > + > +CFLAGS_armada_trace.o := -I$(src) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: use atomic helper commit
Hi Inki, Thank you for the patch. On Monday 16 Jan 2017 18:13:22 Inki Dae wrote: > This patch relpaces specific atomic commit function > with atomic helper commit one, which also includes > atomic_commit_tail callback for Exynos SoC becasue > crtc devices on Exynos SoC uses power domain device > so drm_atomic_helper_commit_planes should be called > after drm_atomic_helper_commit_modeset_enables. Please note that drm_atomic_helper_commit() is currently broken, its async commit support is subject to a race condition. Maarten's "[PATCH v3 0/7] drm/atomic: Add accessor macros for all atomic state" patch series is an attempt to fix that, I'll try to review it ASAP. > Signed-off-by: Inki Dae> --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 9 ++- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 110 +--- > drivers/gpu/drm/exynos/exynos_drm_fb.c | 25 ++- > 3 files changed, 33 insertions(+), 111 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c index 2530bf5..47da612 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -39,6 +39,14 @@ static void exynos_drm_crtc_disable(struct drm_crtc > *crtc) > > if (exynos_crtc->ops->disable) > exynos_crtc->ops->disable(exynos_crtc); > + > + if (crtc->state->event && !crtc->state->active) { > + spin_lock_irq(>dev->event_lock); > + drm_crtc_send_vblank_event(crtc, crtc->state->event); > + spin_unlock_irq(>dev->event_lock); > + > + crtc->state->event = NULL; > + } > } > > static void > @@ -94,7 +102,6 @@ static void exynos_crtc_atomic_flush(struct drm_crtc > *crtc, drm_crtc_send_vblank_event(crtc, event); > spin_unlock_irqrestore(>dev->event_lock, flags); > } > - > } > > static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = { > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 3ec0535..9d0df00 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -38,56 +38,6 @@ > #define DRIVER_MAJOR 1 > #define DRIVER_MINOR 0 > > -struct exynos_atomic_commit { > - struct work_struct work; > - struct drm_device *dev; > - struct drm_atomic_state *state; > - u32 crtcs; > -}; > - > -static void exynos_atomic_commit_complete(struct exynos_atomic_commit > *commit) -{ > - struct drm_device *dev = commit->dev; > - struct exynos_drm_private *priv = dev->dev_private; > - struct drm_atomic_state *state = commit->state; > - > - drm_atomic_helper_commit_modeset_disables(dev, state); > - > - drm_atomic_helper_commit_modeset_enables(dev, state); > - > - /* > - * Exynos can't update planes with CRTCs and encoders disabled, > - * its updates routines, specially for FIMD, requires the clocks > - * to be enabled. So it is necessary to handle the modeset operations > - * *before* the commit_planes() step, this way it will always > - * have the relevant clocks enabled to perform the update. > - */ > - > - drm_atomic_helper_commit_planes(dev, state, 0); > - > - drm_atomic_helper_wait_for_vblanks(dev, state); > - > - drm_atomic_helper_cleanup_planes(dev, state); > - > - drm_atomic_state_put(state); > - > - spin_lock(>lock); > - priv->pending &= ~commit->crtcs; > - spin_unlock(>lock); > - > - wake_up_all(>wait); > - > - kfree(commit); > -} > - > -static void exynos_drm_atomic_work(struct work_struct *work) > -{ > - struct exynos_atomic_commit *commit = container_of(work, > - struct exynos_atomic_commit, work); > - > - exynos_atomic_commit_complete(commit); > -} > - > static struct device *exynos_drm_get_dma_device(void); > > static int exynos_drm_load(struct drm_device *dev, unsigned long flags) > @@ -202,65 +152,6 @@ static void exynos_drm_unload(struct drm_device *dev) > dev->dev_private = NULL; > } > > -static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs) > -{ > - bool pending; > - > - spin_lock(>lock); > - pending = priv->pending & crtcs; > - spin_unlock(>lock); > - > - return pending; > -} > - > -int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state > *state, - bool nonblock) > -{ > - struct exynos_drm_private *priv = dev->dev_private; > - struct exynos_atomic_commit *commit; > - struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > - int i, ret; > - > - commit = kzalloc(sizeof(*commit), GFP_KERNEL); > - if (!commit) > - return -ENOMEM; > - > - ret = drm_atomic_helper_prepare_planes(dev, state); > - if (ret) { > - kfree(commit); > - return ret; > - } > - > -
Re: [PATCH v1 1/7] dt-bindings: display: add STM32 LTDC driver
Hi Yannick, Thank you for the patch. On Monday 16 Jan 2017 14:28:58 Yannick Fertre wrote: > Signed-off-by: Yannick Fertre> --- > .../devicetree/bindings/display/st,ltdc.txt| 57 ++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/st,ltdc.txt > > diff --git a/Documentation/devicetree/bindings/display/st,ltdc.txt > b/Documentation/devicetree/bindings/display/st,ltdc.txt new file mode > 100644 > index 000..20e89da > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/st,ltdc.txt > @@ -0,0 +1,57 @@ > +* STMicroelectronics STM32 lcd-tft display controller > + > +- st-display-subsystem: Master device for DRM sub-components > + This device must be the parent of all the sub-components and is > responsible > + of bind them. Why do you need this ? At a quick glance the ltdc node should be enough. > + Required properties: > + - compatible: "st,display-subsystem" > + - ranges: to allow probing of subdevices > + > +- ltdc_host: lcd-tft display controller host > + must be a sub-node of st-display-subsystem > + Required properties: > + - compatible: "st,ltdc" > + - reg: Physical base address of the IP registers and length of memory > mapped region. > + - clocks: from common clock binding: handle hardware IP needed clocks, > the > +number of clocks may depend of the SoC type. > +See ../clocks/clock-bindings.txt for details. > + - clock-names: names of the clocks listed in clocks property in the same > +order. You need to define the required/optional clocks with their names here. If they vary depending on the SoC, the DT bindings document need to list them for each SoC. > + - resets: resets to be used by the device > +See ../reset/reset.txt for details. > + - reset-names: names of the resets listed in resets property in the same > +order. > + Required nodes: > +- Video port for RGB output. > + > +Example: > + > +/ { > + ... > + soc { > + ... > + st-display-subsystem { > + compatible = "st,display-subsystem"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + dma-ranges; > + > + ltdc_host: stm32-ltdc@40016800 { > + compatible = "st,ltdc"; > + reg = <0x40016800 0x200>; > + interrupts = <88>, <89>; > + resets = < 314>; > + clocks = < 1 8>; > + clock-names = "clk-lcd"; > + status = "disabled"; > + > + port { > + ltdc_out_rgb: endpoint { > + }; > + }; > + }; > + }; > + ... > + }; > +}; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state
Hi Andrey, 2017-01-16 Andrey Grodzovsky: > Allows using atomic flip helpers for drivers > using ASYNC flip. > Remove ASYNC_FLIP restriction in helpers and > caches the page flip flags in drm_plane_state > to be used in the low level drivers. > > Signed-off-by: Andrey Grodzovsky > --- > drivers/gpu/drm/drm_atomic_helper.c | 10 +++--- > include/drm/drm_plane.h | 8 > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index a4e5477..f83dc43 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2737,7 +2737,8 @@ static int page_flip_common( > struct drm_atomic_state *state, > struct drm_crtc *crtc, > struct drm_framebuffer *fb, > - struct drm_pending_vblank_event *event) > + struct drm_pending_vblank_event *event, > + uint32_t flags) Did you build this patch? It is changing the signature of page_flip_common() but no changes to the callers. Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5 v3] drm/bridge: adv7511: Reuse __adv7511_power_on/off() when probing EDID
On Mon, Jan 16, 2017 at 8:03 AM, Laurent Pinchartwrote: > Hi John, > > Thank you for the patch. > > On Tuesday 03 Jan 2017 11:41:42 John Stultz wrote: >> I've found that by just turning the chip on and off via the >> POWER_DOWN register, I end up getting i2c_transfer errors >> on HiKey. >> >> Investigating further, it seems some of the register state >> in the regmap cache is somehow getting lost. Using the logic >> in __adv7511_power_on/off() which syncs and dirtys the cache >> avoids this issue. >> >> Thus this patch changes the EDID probing logic so that we >> re-use the __adv7511_power_on/off() calls. > > regcache_sync() is quite costly as it will write a bunch of registers. > Wouldn't it be more efficient to only write the registers that are needed for > EDID access ? So yes, you've mentioned this concern before, and I did spend some time to narrow which lost-register state (0x43 - ADV7511_REG_EDID_I2C_ADDR) was causing the trouble with i2c trasnfer errors I was seeing: https://lkml.org/lkml/2016/11/22/677 However, I didn't get much feedback on that, and it seems (to me at least) concerning that we are losing the underlying state of a register in the cache, so just syncing that one register back to the hardware might solve the issue I was seeing, but I worry what other registers might also be out of sync. The comment above the regmap_sync in adv7511_power_on after all states: "Most of the registers are reset during power down or when HPD is low." So it seems like if we're setting the power down (and setting HPD in cases where Archit had a patch to add HPD pulsing to the adv7511_get_modes path), it seems reasonable to do the same regmap_sync()? But, I'm not really picky here, and I'm very open to other approaches (including something like the patch in the link above) if you have suggestions/preferences. I just want it to work reliably on my hardware. :) And just so I can better understand it, can you explain some about the impact of your efficiency concerns? thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/armada: Include current dir on CFLAGS for armada trace
From: Gustavo PadovanOtherwise compilation fails like this: In file included from drivers/gpu/drm/armada/armada_trace.h:66:0, from drivers/gpu/drm/armada/armada_trace.c:3: ./include/trace/define_trace.h:88:43: fatal error: ./armada_trace.h: No such file or directory compilation terminated. Signed-off-by: Gustavo Padovan --- drivers/gpu/drm/armada/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/armada/Makefile b/drivers/gpu/drm/armada/Makefile index a18f156..64c0b45 100644 --- a/drivers/gpu/drm/armada/Makefile +++ b/drivers/gpu/drm/armada/Makefile @@ -4,3 +4,5 @@ armada-y+= armada_510.o armada-$(CONFIG_DEBUG_FS) += armada_debugfs.o obj-$(CONFIG_DRM_ARMADA) := armada.o + +CFLAGS_armada_trace.o := -I$(src) -- 2.5.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Clean up the 1366x768 fixup codes
On Mon, 16 Jan 2017 20:16:04 +0100, Gustavo Padovan wrote: > > Hi Takashi, > > 2017-01-16 Takashi Iwai: > > > This is just a cleanup, no functional change. > > > > The fixup code for 1366x768 in drm_mode_create_from_cmdline_mode() is > > basically a copy of the existing code in drm_edid.c. Make the latter > > code public so that it can be called from the former function. > > > > Signed-off-by: Takashi Iwai > > --- > > drivers/gpu/drm/drm_crtc_internal.h | 3 +++ > > drivers/gpu/drm/drm_edid.c | 6 +++--- > > drivers/gpu/drm/drm_modes.c | 9 ++--- > > 3 files changed, 8 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h > > b/drivers/gpu/drm/drm_crtc_internal.h > > index cdf6860c9d22..01bde7103ad6 100644 > > --- a/drivers/gpu/drm/drm_crtc_internal.h > > +++ b/drivers/gpu/drm/drm_crtc_internal.h > > @@ -199,3 +199,6 @@ int drm_mode_cursor2_ioctl(struct drm_device *dev, > >void *data, struct drm_file *file_priv); > > int drm_mode_page_flip_ioctl(struct drm_device *dev, > > void *data, struct drm_file *file_priv); > > + > > +/* drm_edid.c */ > > +void drm_mode_fixup_1366x768(struct drm_display_mode *mode); > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > > index 336be31ff3de..739a19cb27d9 100644 > > --- a/drivers/gpu/drm/drm_edid.c > > +++ b/drivers/gpu/drm/drm_edid.c > > @@ -2152,7 +2152,7 @@ drm_dmt_modes_for_range(struct drm_connector > > *connector, struct edid *edid, > > /* fix up 1366x768 mode from 1368x768; > > * GFT/CVT can't express 1366 width which isn't dividable by 8 > > */ > > -static void fixup_mode_1366x768(struct drm_display_mode *mode) > > +void drm_mode_fixup_1366x768(struct drm_display_mode *mode) > > { > > if (mode->hdisplay == 1368 && mode->vdisplay == 768) { > > mode->hdisplay = 1366; > > @@ -2176,7 +2176,7 @@ drm_gtf_modes_for_range(struct drm_connector > > *connector, struct edid *edid, > > if (!newmode) > > return modes; > > > > - fixup_mode_1366x768(newmode); > > + drm_mode_fixup_1366x768(newmode); > > if (!mode_in_range(newmode, edid, timing) || > > !valid_inferred_mode(connector, newmode)) { > > drm_mode_destroy(dev, newmode); > > @@ -2205,7 +2205,7 @@ drm_cvt_modes_for_range(struct drm_connector > > *connector, struct edid *edid, > > if (!newmode) > > return modes; > > > > - fixup_mode_1366x768(newmode); > > + drm_mode_fixup_1366x768(newmode); > > if (!mode_in_range(newmode, edid, timing) || > > !valid_inferred_mode(connector, newmode)) { > > drm_mode_destroy(dev, newmode); > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index e6b19bc9021a..860f4d1ffbde 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -1461,13 +1461,8 @@ drm_mode_create_from_cmdline_mode(struct drm_device > > *dev, > > > > mode->type |= DRM_MODE_TYPE_USERDEF; > > /* fix up 1368x768: GFT/CVT can't express 1366 width due to alignment */ > > - if (cmd->xres == 1366 && mode->hdisplay == 1368) { > > - mode->hdisplay = 1366; > > - mode->hsync_start--; > > - mode->hsync_end--; > > - drm_mode_set_name(mode); > > - } > > - drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); > > At a glance removing the call to drm_mode_set_crtcinfo() seems a > functional change to me. I'm not sure why you removed it. Oops, of course it's wrong. It shouldn't have been deleted. Will resubmit the proper patch. Thanks for catching! Takashi ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5 v3] drm/bridge: adv7511: Switch to using drm_kms_helper_hotplug_event()
On Mon, Jan 16, 2017 at 7:56 AM, Laurent Pinchartwrote: > On Monday 16 Jan 2017 17:47:46 Laurent Pinchart wrote: >> Hi John, >> >> Thank you for the patch. >> >> On Tuesday 03 Jan 2017 11:41:39 John Stultz wrote: >> > In chasing down a previous issue with EDID probing from calling >> > drm_helper_hpd_irq_event() from irq context, Laurent noticed >> > that the DRM documentation suggests that >> > drm_kms_helper_hotplug_event() should be used instead. >> > >> > Thus this patch replaces drm_helper_hpd_irq_event() with >> > drm_kms_helper_hotplug_event(), which requires we update the >> > connector.status entry and only call _hotplug_event() when the >> > status changes. >> > >> > Cc: David Airlie >> > Cc: Archit Taneja >> > Cc: Wolfram Sang >> > Cc: Lars-Peter Clausen >> > Cc: Laurent Pinchart >> > Cc: dri-devel@lists.freedesktop.org >> > Signed-off-by: John Stultz >> > --- >> > v3: Update connector.status value and only call __hotplug_event() >> > >> > when that status changes, as suggested by Laurent. >> > >> > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 16 +++- >> > 1 file changed, 15 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 4fcea44..d93d66f >> > 100644 >> > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> > @@ -405,8 +405,22 @@ static bool adv7511_hpd(struct adv7511 *adv7511) >> > >> > static void adv7511_hpd_work(struct work_struct *work) >> > { >> > >> > struct adv7511 *adv7511 = container_of(work, struct adv7511, >> >> hpd_work); >> >> > + enum drm_connector_status status; >> > + unsigned int val; >> > + int ret; >> > + >> > + ret = regmap_read(adv7511->regmap, ADV7511_REG_STATUS, ); >> > + if (ret < 0) >> > + status = connector_status_disconnected; >> > + else if (val & ADV7511_STATUS_HPD) >> > + status = connector_status_connected; >> > + else >> > + status = connector_status_disconnected; >> > + >> > + if (adv7511->connector.status != status) >> > + drm_kms_helper_hotplug_event(adv7511->connector.dev); >> > >> > - drm_helper_hpd_irq_event(adv7511->connector.dev); >> > + adv7511->connector.status = status; >> >> Shouldn't you update the status before calling >> drm_kms_helper_hotplug_event() ? Doing it after not only creates a small >> race condition as >> drm_kms_helper_hotplug_event() sends an event to userspace that could result >> in an ioctl call to retrieve the status, but the status is also checked by >> drm_setup_crtcs() called by drm_fb_helper_hotplug_event(). > > With > > if (adv7511->connector.status != status) { > adv7511->connector.status = status; > drm_kms_helper_hotplug_event(adv7511->connector.dev); > } > > Reviewed-by: Laurent Pinchart > Tested-by: Laurent Pinchart Thanks so much for catching this! I'll respin the patches with this fix and resend a v4. thanks again! -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Clean up the 1366x768 fixup codes
Hi Takashi, 2017-01-16 Takashi Iwai: > This is just a cleanup, no functional change. > > The fixup code for 1366x768 in drm_mode_create_from_cmdline_mode() is > basically a copy of the existing code in drm_edid.c. Make the latter > code public so that it can be called from the former function. > > Signed-off-by: Takashi Iwai > --- > drivers/gpu/drm/drm_crtc_internal.h | 3 +++ > drivers/gpu/drm/drm_edid.c | 6 +++--- > drivers/gpu/drm/drm_modes.c | 9 ++--- > 3 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h > b/drivers/gpu/drm/drm_crtc_internal.h > index cdf6860c9d22..01bde7103ad6 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -199,3 +199,6 @@ int drm_mode_cursor2_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > int drm_mode_page_flip_ioctl(struct drm_device *dev, >void *data, struct drm_file *file_priv); > + > +/* drm_edid.c */ > +void drm_mode_fixup_1366x768(struct drm_display_mode *mode); > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 336be31ff3de..739a19cb27d9 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2152,7 +2152,7 @@ drm_dmt_modes_for_range(struct drm_connector > *connector, struct edid *edid, > /* fix up 1366x768 mode from 1368x768; > * GFT/CVT can't express 1366 width which isn't dividable by 8 > */ > -static void fixup_mode_1366x768(struct drm_display_mode *mode) > +void drm_mode_fixup_1366x768(struct drm_display_mode *mode) > { > if (mode->hdisplay == 1368 && mode->vdisplay == 768) { > mode->hdisplay = 1366; > @@ -2176,7 +2176,7 @@ drm_gtf_modes_for_range(struct drm_connector > *connector, struct edid *edid, > if (!newmode) > return modes; > > - fixup_mode_1366x768(newmode); > + drm_mode_fixup_1366x768(newmode); > if (!mode_in_range(newmode, edid, timing) || > !valid_inferred_mode(connector, newmode)) { > drm_mode_destroy(dev, newmode); > @@ -2205,7 +2205,7 @@ drm_cvt_modes_for_range(struct drm_connector > *connector, struct edid *edid, > if (!newmode) > return modes; > > - fixup_mode_1366x768(newmode); > + drm_mode_fixup_1366x768(newmode); > if (!mode_in_range(newmode, edid, timing) || > !valid_inferred_mode(connector, newmode)) { > drm_mode_destroy(dev, newmode); > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index e6b19bc9021a..860f4d1ffbde 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -1461,13 +1461,8 @@ drm_mode_create_from_cmdline_mode(struct drm_device > *dev, > > mode->type |= DRM_MODE_TYPE_USERDEF; > /* fix up 1368x768: GFT/CVT can't express 1366 width due to alignment */ > - if (cmd->xres == 1366 && mode->hdisplay == 1368) { > - mode->hdisplay = 1366; > - mode->hsync_start--; > - mode->hsync_end--; > - drm_mode_set_name(mode); > - } > - drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V); At a glance removing the call to drm_mode_set_crtcinfo() seems a functional change to me. I'm not sure why you removed it. Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: use atomic helper commit
Hi Inki, 2017-01-16 Inki Dae: > This patch relpaces specific atomic commit function > with atomic helper commit one, which also includes > atomic_commit_tail callback for Exynos SoC becasue > crtc devices on Exynos SoC uses power domain device > so drm_atomic_helper_commit_planes should be called > after drm_atomic_helper_commit_modeset_enables. Indeed, the commit message needs fixing. > > Signed-off-by: Inki Dae > --- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 9 ++- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 110 > +-- > drivers/gpu/drm/exynos/exynos_drm_fb.c | 25 ++- > 3 files changed, 33 insertions(+), 111 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > index 2530bf5..47da612 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c > @@ -39,6 +39,14 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc) > > if (exynos_crtc->ops->disable) > exynos_crtc->ops->disable(exynos_crtc); > + > + if (crtc->state->event && !crtc->state->active) { > + spin_lock_irq(>dev->event_lock); > + drm_crtc_send_vblank_event(crtc, crtc->state->event); > + spin_unlock_irq(>dev->event_lock); > + > + crtc->state->event = NULL; > + } > } > > static void > @@ -94,7 +102,6 @@ static void exynos_crtc_atomic_flush(struct drm_crtc *crtc, > drm_crtc_send_vblank_event(crtc, event); > spin_unlock_irqrestore(>dev->event_lock, flags); > } > - Nitpick: I wouldn't include changes like this in the patch. > } > > static const struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = { > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 3ec0535..9d0df00 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -38,56 +38,6 @@ > #define DRIVER_MAJOR 1 > #define DRIVER_MINOR 0 > > -struct exynos_atomic_commit { > - struct work_struct work; > - struct drm_device *dev; > - struct drm_atomic_state *state; > - u32 crtcs; > -}; > - > -static void exynos_atomic_commit_complete(struct exynos_atomic_commit > *commit) > -{ > - struct drm_device *dev = commit->dev; > - struct exynos_drm_private *priv = dev->dev_private; > - struct drm_atomic_state *state = commit->state; > - > - drm_atomic_helper_commit_modeset_disables(dev, state); > - > - drm_atomic_helper_commit_modeset_enables(dev, state); > - > - /* > - * Exynos can't update planes with CRTCs and encoders disabled, > - * its updates routines, specially for FIMD, requires the clocks > - * to be enabled. So it is necessary to handle the modeset operations > - * *before* the commit_planes() step, this way it will always > - * have the relevant clocks enabled to perform the update. > - */ Please move this comment to the commit_tail function instead of deleting it. > - > - drm_atomic_helper_commit_planes(dev, state, 0); > - > - drm_atomic_helper_wait_for_vblanks(dev, state); > - > - drm_atomic_helper_cleanup_planes(dev, state); > - > - drm_atomic_state_put(state); > - > - spin_lock(>lock); > - priv->pending &= ~commit->crtcs; > - spin_unlock(>lock); > - > - wake_up_all(>wait); > - > - kfree(commit); > -} > - > -static void exynos_drm_atomic_work(struct work_struct *work) > -{ > - struct exynos_atomic_commit *commit = container_of(work, > - struct exynos_atomic_commit, work); > - > - exynos_atomic_commit_complete(commit); > -} > - > static struct device *exynos_drm_get_dma_device(void); > > static int exynos_drm_load(struct drm_device *dev, unsigned long flags) > @@ -202,65 +152,6 @@ static void exynos_drm_unload(struct drm_device *dev) > dev->dev_private = NULL; > } > > -static int commit_is_pending(struct exynos_drm_private *priv, u32 crtcs) > -{ > - bool pending; > - > - spin_lock(>lock); > - pending = priv->pending & crtcs; > - spin_unlock(>lock); > - > - return pending; > -} > - > -int exynos_atomic_commit(struct drm_device *dev, struct drm_atomic_state > *state, > - bool nonblock) > -{ > - struct exynos_drm_private *priv = dev->dev_private; > - struct exynos_atomic_commit *commit; > - struct drm_crtc *crtc; > - struct drm_crtc_state *crtc_state; > - int i, ret; > - > - commit = kzalloc(sizeof(*commit), GFP_KERNEL); > - if (!commit) > - return -ENOMEM; > - > - ret = drm_atomic_helper_prepare_planes(dev, state); > - if (ret) { > - kfree(commit); > - return ret; > - } > - > - /* This is the point of no return */ > - > - INIT_WORK(>work,
[Bug 99310] Ubuntu 16.04/16.10 AMD Radeonsi - wrong colors (oibaf ppa) in XFCE and eog.
https://bugs.freedesktop.org/show_bug.cgi?id=99310 --- Comment #2 from baptiste.fa...@orange.fr--- Same problem with me : reddish icons and blue tint for jpegs. Using ubuntu 16.10 with mesa 13.0 (paolo dias package) and a amd r390. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel