Message ID | 1439289071-23356-2-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
For both patches, Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> On Tue, 2015-08-11 at 12:31 +0200, Maarten Lankhorst wrote: > This patch is based on the upstream commit 5ac1c4bcf073ad and amended > for v4.2 to make sure it works as intended. > > Repeated calls to begin_crtc_commit can cause warnings like this: > [ 169.127746] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616 > [ 169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip > [ 169.127840] 3 locks held by kms_flip/1947: > [ 169.127843] #0: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffff814774bc>] > __drm_modeset_lock_all+0x9c/0x130 > [ 169.127860] #1: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffff814774cd>] > __drm_modeset_lock_all+0xad/0x130 > [ 169.127870] #2: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffff81477178>] > drm_modeset_lock+0x38/0x110 > [ 169.127879] irq event stamp: 665690 > [ 169.127882] hardirqs last enabled at (665689): [<ffffffff817ffdb5>] > _raw_spin_unlock_irqrestore+0x55/0x70 > [ 169.127889] hardirqs last disabled at (665690): [<ffffffffc0197a23>] > intel_pipe_update_start+0x113/0x5c0 [i915] > [ 169.127936] softirqs last enabled at (665470): [<ffffffff8108a766>] __do_softirq+0x236/0x650 > [ 169.127942] softirqs last disabled at (665465): [<ffffffff8108ae75>] irq_exit+0xc5/0xd0 > [ 169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+ #4039 > [ 169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 > [ 169.127957] ffff8800c49036f0 ffff8800cde5fa28 ffffffff817f6907 0000000080000001 > [ 169.127964] 0000000000000000 ffff8800cde5fa58 ffffffff810aebed 0000000000000046 > [ 169.127970] ffffffff81c5d518 0000000000000268 0000000000000000 ffff8800cde5fa88 > [ 169.127981] Call Trace: > [ 169.127992] [<ffffffff817f6907>] dump_stack+0x4f/0x7b > [ 169.128001] [<ffffffff810aebed>] ___might_sleep+0x16d/0x270 > [ 169.128008] [<ffffffff810aed38>] __might_sleep+0x48/0x90 > [ 169.128017] [<ffffffff817fc359>] mutex_lock_nested+0x29/0x410 > [ 169.128073] [<ffffffffc01635f0>] ? vgpu_write64+0x220/0x220 [i915] > [ 169.128138] [<ffffffffc017fddf>] ? ironlake_update_primary_plane+0x2ff/0x410 [i915] > [ 169.128198] [<ffffffffc0190e75>] intel_frontbuffer_flush+0x25/0x70 [i915] > [ 169.128253] [<ffffffffc01831ac>] intel_finish_crtc_commit+0x4c/0x180 [i915] > [ 169.128279] [<ffffffffc00784ac>] drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper] > [ 169.128338] [<ffffffffc0184264>] __intel_set_mode+0x684/0x830 [i915] > [ 169.128378] [<ffffffffc018a84a>] intel_crtc_set_config+0x49a/0x620 [i915] > [ 169.128385] [<ffffffff817fdd39>] ? mutex_unlock+0x9/0x10 > [ 169.128391] [<ffffffff81467b69>] drm_mode_set_config_internal+0x69/0x120 > [ 169.128398] [<ffffffff8119b547>] ? might_fault+0x57/0xb0 > [ 169.128403] [<ffffffff8146bf93>] drm_mode_setcrtc+0x253/0x620 > [ 169.128409] [<ffffffff8145c600>] drm_ioctl+0x1a0/0x6a0 > [ 169.128415] [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 > [ 169.128424] [<ffffffff811e9ab8>] do_vfs_ioctl+0x2f8/0x530 > [ 169.128429] [<ffffffff810d0fcd>] ? trace_hardirqs_on+0xd/0x10 > [ 169.128435] [<ffffffff812e7676>] ? selinux_file_ioctl+0x56/0x100 > [ 169.128439] [<ffffffff811e9d71>] SyS_ioctl+0x81/0xa0 > [ 169.128445] [<ffffffff81800697>] system_call_fastpath+0x12/0x6f > > Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc. > > The problem here was that the drm_atomic_helper_commit_planes() helper > we were using was basically designed to do > > begin_crtc_commit(crtc #1) > begin_crtc_commit(crtc #2) > ... > commit all planes > finish_crtc_commit(crtc #1) > finish_crtc_commit(crtc #2) > > The problem here is that since our hardware relies on vblank evasion, > our CRTC 'begin' function waits until we're out of the danger zone in > which register writes might wind up straddling the vblank, then disables > interrupts; our 'finish' function re-enables interrupts after the > registers have been written. The expectation is that the operations between > 'begin' and 'end' must be performed without sleeping (since interrupts > are disabled) and should happen as quickly as possible. By clumping all > of the 'begin' calls together, we introducing a couple problems: > * Subsequent 'begin' invocations might sleep (which is illegal) > * The first 'begin' ensured that we were far enough from the vblank that > we could write our registers safely and ensure they all fell within > the same frame. Adding extra delay waiting for subsequent CRTC's > wasn't accounted for and could put us back into the 'danger zone' for > CRTC #1. > > This commit solves the problem by using a new helper that allows an > order of operations like: > > for each crtc { > begin_crtc_commit(crtc) // sleep (maybe), then disable interrupts > commit planes for this specific CRTC > end_crtc_commit(crtc) // reenable interrupts > } > > so that sleeps will only be performed while interrupts are enabled and > we can be sure that registers for a CRTC will be written immediately > once we know we're in the safe zone. > > The crtc->config->base.crtc update may seem unrelated, but the helper > will use it to obtain the crtc for the state. Without the update it > will dereference NULL and crash. > > Changes since v1: > - Use Matt Roper's commit message. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > Cc: stable@vger.kernel.org #v4.2 > References: https://bugs.freedesktop.org/show_bug.cgi?id=90398 > --- > drivers/gpu/drm/i915/intel_atomic.c | 45 +++++++----------------------------- > drivers/gpu/drm/i915/intel_display.c | 11 +++++---- > 2 files changed, 14 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 7ed8033aae60..8e35e0d013df 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -129,8 +129,9 @@ int intel_atomic_commit(struct drm_device *dev, > struct drm_atomic_state *state, > bool async) > { > - int ret; > - int i; > + struct drm_crtc_state *crtc_state; > + struct drm_crtc *crtc; > + int ret, i; > > if (async) { > DRM_DEBUG_KMS("i915 does not yet support async commit\n"); > @@ -142,48 +143,18 @@ int intel_atomic_commit(struct drm_device *dev, > return ret; > > /* Point of no return */ > - > - /* > - * FIXME: The proper sequence here will eventually be: > - * > - * drm_atomic_helper_swap_state(dev, state) > - * drm_atomic_helper_commit_modeset_disables(dev, state); > - * drm_atomic_helper_commit_planes(dev, state); > - * drm_atomic_helper_commit_modeset_enables(dev, state); > - * drm_atomic_helper_wait_for_vblanks(dev, state); > - * drm_atomic_helper_cleanup_planes(dev, state); > - * drm_atomic_state_free(state); > - * > - * once we have full atomic modeset. For now, just manually update > - * plane states to avoid clobbering good states with dummy states > - * while nuclear pageflipping. > - */ > - for (i = 0; i < dev->mode_config.num_total_plane; i++) { > - struct drm_plane *plane = state->planes[i]; > - > - if (!plane) > - continue; > - > - plane->state->state = state; > - swap(state->plane_states[i], plane->state); > - plane->state->state = NULL; > - } > + drm_atomic_helper_swap_state(dev, state); > > /* swap crtc_scaler_state */ > - for (i = 0; i < dev->mode_config.num_crtc; i++) { > - struct drm_crtc *crtc = state->crtcs[i]; > - if (!crtc) { > - continue; > - } > - > - to_intel_crtc(crtc)->config->scaler_state = > - to_intel_crtc_state(state->crtc_states[i])->scaler_state; > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); > > if (INTEL_INFO(dev)->gen >= 9) > skl_detach_scalers(to_intel_crtc(crtc)); > + > + drm_atomic_helper_commit_planes_on_crtc(crtc_state); > } > > - drm_atomic_helper_commit_planes(dev, state); > drm_atomic_helper_wait_for_vblanks(dev, state); > drm_atomic_helper_cleanup_planes(dev, state); > drm_atomic_state_free(state); > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c2579ded0c36..b920f88ccff8 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12624,17 +12624,17 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, > > modeset_update_crtc_power_domains(state); > > - drm_atomic_helper_commit_planes(dev, state); > - > /* Now enable the clocks, plane, pipe, and connectors that we set up. */ > for_each_crtc_in_state(state, crtc, crtc_state, i) { > - if (!needs_modeset(crtc->state) || !crtc->state->enable) > + if (!needs_modeset(crtc->state) || !crtc->state->enable) { > + drm_atomic_helper_commit_planes_on_crtc(crtc_state); > continue; > + } > > update_scanline_offset(to_intel_crtc(crtc)); > > dev_priv->display.crtc_enable(crtc); > - intel_crtc_enable_planes(crtc); > + drm_atomic_helper_commit_planes_on_crtc(crtc_state); > } > > /* FIXME: add subpixel order */ > @@ -13267,7 +13267,7 @@ intel_check_primary_plane(struct drm_plane *plane, > if (IS_BROADWELL(dev)) > intel_crtc->atomic.wait_vblank = true; > > - if (crtc_state && !needs_modeset(&crtc_state->base)) > + if (crtc_state) > intel_crtc->atomic.post_enable_primary = true; > } > > @@ -15002,6 +15002,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > struct intel_plane_state *plane_state; > > memset(crtc->config, 0, sizeof(*crtc->config)); > + crtc->config->base.crtc = &crtc->base; > > crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE; >
On Wed, 12 Aug 2015, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote: > For both patches, > > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com> Both pushed to drm-intel-fixes, thanks for the patches and review. BR, Jani. > > On Tue, 2015-08-11 at 12:31 +0200, Maarten Lankhorst wrote: >> This patch is based on the upstream commit 5ac1c4bcf073ad and amended >> for v4.2 to make sure it works as intended. >> >> Repeated calls to begin_crtc_commit can cause warnings like this: >> [ 169.127746] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616 >> [ 169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip >> [ 169.127840] 3 locks held by kms_flip/1947: >> [ 169.127843] #0: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffff814774bc>] >> __drm_modeset_lock_all+0x9c/0x130 >> [ 169.127860] #1: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffff814774cd>] >> __drm_modeset_lock_all+0xad/0x130 >> [ 169.127870] #2: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffff81477178>] >> drm_modeset_lock+0x38/0x110 >> [ 169.127879] irq event stamp: 665690 >> [ 169.127882] hardirqs last enabled at (665689): [<ffffffff817ffdb5>] >> _raw_spin_unlock_irqrestore+0x55/0x70 >> [ 169.127889] hardirqs last disabled at (665690): [<ffffffffc0197a23>] >> intel_pipe_update_start+0x113/0x5c0 [i915] >> [ 169.127936] softirqs last enabled at (665470): [<ffffffff8108a766>] __do_softirq+0x236/0x650 >> [ 169.127942] softirqs last disabled at (665465): [<ffffffff8108ae75>] irq_exit+0xc5/0xd0 >> [ 169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+ #4039 >> [ 169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 >> [ 169.127957] ffff8800c49036f0 ffff8800cde5fa28 ffffffff817f6907 0000000080000001 >> [ 169.127964] 0000000000000000 ffff8800cde5fa58 ffffffff810aebed 0000000000000046 >> [ 169.127970] ffffffff81c5d518 0000000000000268 0000000000000000 ffff8800cde5fa88 >> [ 169.127981] Call Trace: >> [ 169.127992] [<ffffffff817f6907>] dump_stack+0x4f/0x7b >> [ 169.128001] [<ffffffff810aebed>] ___might_sleep+0x16d/0x270 >> [ 169.128008] [<ffffffff810aed38>] __might_sleep+0x48/0x90 >> [ 169.128017] [<ffffffff817fc359>] mutex_lock_nested+0x29/0x410 >> [ 169.128073] [<ffffffffc01635f0>] ? vgpu_write64+0x220/0x220 [i915] >> [ 169.128138] [<ffffffffc017fddf>] ? ironlake_update_primary_plane+0x2ff/0x410 [i915] >> [ 169.128198] [<ffffffffc0190e75>] intel_frontbuffer_flush+0x25/0x70 [i915] >> [ 169.128253] [<ffffffffc01831ac>] intel_finish_crtc_commit+0x4c/0x180 [i915] >> [ 169.128279] [<ffffffffc00784ac>] drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper] >> [ 169.128338] [<ffffffffc0184264>] __intel_set_mode+0x684/0x830 [i915] >> [ 169.128378] [<ffffffffc018a84a>] intel_crtc_set_config+0x49a/0x620 [i915] >> [ 169.128385] [<ffffffff817fdd39>] ? mutex_unlock+0x9/0x10 >> [ 169.128391] [<ffffffff81467b69>] drm_mode_set_config_internal+0x69/0x120 >> [ 169.128398] [<ffffffff8119b547>] ? might_fault+0x57/0xb0 >> [ 169.128403] [<ffffffff8146bf93>] drm_mode_setcrtc+0x253/0x620 >> [ 169.128409] [<ffffffff8145c600>] drm_ioctl+0x1a0/0x6a0 >> [ 169.128415] [<ffffffff810b3b41>] ? get_parent_ip+0x11/0x50 >> [ 169.128424] [<ffffffff811e9ab8>] do_vfs_ioctl+0x2f8/0x530 >> [ 169.128429] [<ffffffff810d0fcd>] ? trace_hardirqs_on+0xd/0x10 >> [ 169.128435] [<ffffffff812e7676>] ? selinux_file_ioctl+0x56/0x100 >> [ 169.128439] [<ffffffff811e9d71>] SyS_ioctl+0x81/0xa0 >> [ 169.128445] [<ffffffff81800697>] system_call_fastpath+0x12/0x6f >> >> Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc. >> >> The problem here was that the drm_atomic_helper_commit_planes() helper >> we were using was basically designed to do >> >> begin_crtc_commit(crtc #1) >> begin_crtc_commit(crtc #2) >> ... >> commit all planes >> finish_crtc_commit(crtc #1) >> finish_crtc_commit(crtc #2) >> >> The problem here is that since our hardware relies on vblank evasion, >> our CRTC 'begin' function waits until we're out of the danger zone in >> which register writes might wind up straddling the vblank, then disables >> interrupts; our 'finish' function re-enables interrupts after the >> registers have been written. The expectation is that the operations between >> 'begin' and 'end' must be performed without sleeping (since interrupts >> are disabled) and should happen as quickly as possible. By clumping all >> of the 'begin' calls together, we introducing a couple problems: >> * Subsequent 'begin' invocations might sleep (which is illegal) >> * The first 'begin' ensured that we were far enough from the vblank that >> we could write our registers safely and ensure they all fell within >> the same frame. Adding extra delay waiting for subsequent CRTC's >> wasn't accounted for and could put us back into the 'danger zone' for >> CRTC #1. >> >> This commit solves the problem by using a new helper that allows an >> order of operations like: >> >> for each crtc { >> begin_crtc_commit(crtc) // sleep (maybe), then disable interrupts >> commit planes for this specific CRTC >> end_crtc_commit(crtc) // reenable interrupts >> } >> >> so that sleeps will only be performed while interrupts are enabled and >> we can be sure that registers for a CRTC will be written immediately >> once we know we're in the safe zone. >> >> The crtc->config->base.crtc update may seem unrelated, but the helper >> will use it to obtain the crtc for the state. Without the update it >> will dereference NULL and crash. >> >> Changes since v1: >> - Use Matt Roper's commit message. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> Cc: stable@vger.kernel.org #v4.2 >> References: https://bugs.freedesktop.org/show_bug.cgi?id=90398 >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 45 +++++++----------------------------- >> drivers/gpu/drm/i915/intel_display.c | 11 +++++---- >> 2 files changed, 14 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c >> index 7ed8033aae60..8e35e0d013df 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -129,8 +129,9 @@ int intel_atomic_commit(struct drm_device *dev, >> struct drm_atomic_state *state, >> bool async) >> { >> - int ret; >> - int i; >> + struct drm_crtc_state *crtc_state; >> + struct drm_crtc *crtc; >> + int ret, i; >> >> if (async) { >> DRM_DEBUG_KMS("i915 does not yet support async commit\n"); >> @@ -142,48 +143,18 @@ int intel_atomic_commit(struct drm_device *dev, >> return ret; >> >> /* Point of no return */ >> - >> - /* >> - * FIXME: The proper sequence here will eventually be: >> - * >> - * drm_atomic_helper_swap_state(dev, state) >> - * drm_atomic_helper_commit_modeset_disables(dev, state); >> - * drm_atomic_helper_commit_planes(dev, state); >> - * drm_atomic_helper_commit_modeset_enables(dev, state); >> - * drm_atomic_helper_wait_for_vblanks(dev, state); >> - * drm_atomic_helper_cleanup_planes(dev, state); >> - * drm_atomic_state_free(state); >> - * >> - * once we have full atomic modeset. For now, just manually update >> - * plane states to avoid clobbering good states with dummy states >> - * while nuclear pageflipping. >> - */ >> - for (i = 0; i < dev->mode_config.num_total_plane; i++) { >> - struct drm_plane *plane = state->planes[i]; >> - >> - if (!plane) >> - continue; >> - >> - plane->state->state = state; >> - swap(state->plane_states[i], plane->state); >> - plane->state->state = NULL; >> - } >> + drm_atomic_helper_swap_state(dev, state); >> >> /* swap crtc_scaler_state */ >> - for (i = 0; i < dev->mode_config.num_crtc; i++) { >> - struct drm_crtc *crtc = state->crtcs[i]; >> - if (!crtc) { >> - continue; >> - } >> - >> - to_intel_crtc(crtc)->config->scaler_state = >> - to_intel_crtc_state(state->crtc_states[i])->scaler_state; >> + for_each_crtc_in_state(state, crtc, crtc_state, i) { >> + to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); >> >> if (INTEL_INFO(dev)->gen >= 9) >> skl_detach_scalers(to_intel_crtc(crtc)); >> + >> + drm_atomic_helper_commit_planes_on_crtc(crtc_state); >> } >> >> - drm_atomic_helper_commit_planes(dev, state); >> drm_atomic_helper_wait_for_vblanks(dev, state); >> drm_atomic_helper_cleanup_planes(dev, state); >> drm_atomic_state_free(state); >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index c2579ded0c36..b920f88ccff8 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12624,17 +12624,17 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, >> >> modeset_update_crtc_power_domains(state); >> >> - drm_atomic_helper_commit_planes(dev, state); >> - >> /* Now enable the clocks, plane, pipe, and connectors that we set up. */ >> for_each_crtc_in_state(state, crtc, crtc_state, i) { >> - if (!needs_modeset(crtc->state) || !crtc->state->enable) >> + if (!needs_modeset(crtc->state) || !crtc->state->enable) { >> + drm_atomic_helper_commit_planes_on_crtc(crtc_state); >> continue; >> + } >> >> update_scanline_offset(to_intel_crtc(crtc)); >> >> dev_priv->display.crtc_enable(crtc); >> - intel_crtc_enable_planes(crtc); >> + drm_atomic_helper_commit_planes_on_crtc(crtc_state); >> } >> >> /* FIXME: add subpixel order */ >> @@ -13267,7 +13267,7 @@ intel_check_primary_plane(struct drm_plane *plane, >> if (IS_BROADWELL(dev)) >> intel_crtc->atomic.wait_vblank = true; >> >> - if (crtc_state && !needs_modeset(&crtc_state->base)) >> + if (crtc_state) >> intel_crtc->atomic.post_enable_primary = true; >> } >> >> @@ -15002,6 +15002,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) >> struct intel_plane_state *plane_state; >> >> memset(crtc->config, 0, sizeof(*crtc->config)); >> + crtc->config->base.crtc = &crtc->base; >> >> crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE; >>
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 7ed8033aae60..8e35e0d013df 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -129,8 +129,9 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool async) { - int ret; - int i; + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + int ret, i; if (async) { DRM_DEBUG_KMS("i915 does not yet support async commit\n"); @@ -142,48 +143,18 @@ int intel_atomic_commit(struct drm_device *dev, return ret; /* Point of no return */ - - /* - * FIXME: The proper sequence here will eventually be: - * - * drm_atomic_helper_swap_state(dev, state) - * drm_atomic_helper_commit_modeset_disables(dev, state); - * drm_atomic_helper_commit_planes(dev, state); - * drm_atomic_helper_commit_modeset_enables(dev, state); - * drm_atomic_helper_wait_for_vblanks(dev, state); - * drm_atomic_helper_cleanup_planes(dev, state); - * drm_atomic_state_free(state); - * - * once we have full atomic modeset. For now, just manually update - * plane states to avoid clobbering good states with dummy states - * while nuclear pageflipping. - */ - for (i = 0; i < dev->mode_config.num_total_plane; i++) { - struct drm_plane *plane = state->planes[i]; - - if (!plane) - continue; - - plane->state->state = state; - swap(state->plane_states[i], plane->state); - plane->state->state = NULL; - } + drm_atomic_helper_swap_state(dev, state); /* swap crtc_scaler_state */ - for (i = 0; i < dev->mode_config.num_crtc; i++) { - struct drm_crtc *crtc = state->crtcs[i]; - if (!crtc) { - continue; - } - - to_intel_crtc(crtc)->config->scaler_state = - to_intel_crtc_state(state->crtc_states[i])->scaler_state; + for_each_crtc_in_state(state, crtc, crtc_state, i) { + to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); if (INTEL_INFO(dev)->gen >= 9) skl_detach_scalers(to_intel_crtc(crtc)); + + drm_atomic_helper_commit_planes_on_crtc(crtc_state); } - drm_atomic_helper_commit_planes(dev, state); drm_atomic_helper_wait_for_vblanks(dev, state); drm_atomic_helper_cleanup_planes(dev, state); drm_atomic_state_free(state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c2579ded0c36..b920f88ccff8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12624,17 +12624,17 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, modeset_update_crtc_power_domains(state); - drm_atomic_helper_commit_planes(dev, state); - /* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!needs_modeset(crtc->state) || !crtc->state->enable) + if (!needs_modeset(crtc->state) || !crtc->state->enable) { + drm_atomic_helper_commit_planes_on_crtc(crtc_state); continue; + } update_scanline_offset(to_intel_crtc(crtc)); dev_priv->display.crtc_enable(crtc); - intel_crtc_enable_planes(crtc); + drm_atomic_helper_commit_planes_on_crtc(crtc_state); } /* FIXME: add subpixel order */ @@ -13267,7 +13267,7 @@ intel_check_primary_plane(struct drm_plane *plane, if (IS_BROADWELL(dev)) intel_crtc->atomic.wait_vblank = true; - if (crtc_state && !needs_modeset(&crtc_state->base)) + if (crtc_state) intel_crtc->atomic.post_enable_primary = true; } @@ -15002,6 +15002,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) struct intel_plane_state *plane_state; memset(crtc->config, 0, sizeof(*crtc->config)); + crtc->config->base.crtc = &crtc->base; crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;