* [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled
@ 2015-05-07 21:19 Matt Roper
2015-05-07 21:28 ` Matt Roper
2015-05-11 6:11 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled shuang.he
0 siblings, 2 replies; 9+ messages in thread
From: Matt Roper @ 2015-05-07 21:19 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
With the recent modeset internal rework, we wind up setting crtc_state->enable
to false, but leave crtc_state->active as true following a
drmModeSetCrtc(fb=0), which is incorrect. This mismatch gets caught by
drm_atomic_crtc_check() and causes subsequent atomic operations (such as plane
updates while the CRTC is disabled) to fail.
Bisect points to
commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Date: Tue Apr 21 17:13:19 2015 +0300
drm/i915: Use atomic helpers for computing changed flags
as the commit that actually triggers the regression.
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: igt/kms_universal_plane
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
My original expectation was that we'd need to be updating the in-flight CRTC
state rather than the committed state as I wound up doing here. However it
looks like our legacy modesets aren't yet converted to the point where they do
state swaps, hence the need to update crtc->state directly.
Ander/Maarten, does this look like the right place to have 'active' updated for
the time being, or should it be moved elsewhere in the modeset pipeline?
drivers/gpu/drm/i915/intel_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c297cdc..be166cf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6128,6 +6128,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
drm_plane_helper_disable(crtc->primary);
+ crtc->state->active = false;
+
/* Update computed state. */
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
if (!connector->encoder || !connector->encoder->crtc)
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled
2015-05-07 21:19 [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled Matt Roper
@ 2015-05-07 21:28 ` Matt Roper
2015-05-07 21:31 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) Matt Roper
2015-05-11 6:11 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled shuang.he
1 sibling, 1 reply; 9+ messages in thread
From: Matt Roper @ 2015-05-07 21:28 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
On Thu, May 07, 2015 at 02:19:07PM -0700, Matt Roper wrote:
> With the recent modeset internal rework, we wind up setting crtc_state->enable
> to false, but leave crtc_state->active as true following a
> drmModeSetCrtc(fb=0), which is incorrect. This mismatch gets caught by
> drm_atomic_crtc_check() and causes subsequent atomic operations (such as plane
> updates while the CRTC is disabled) to fail.
>
> Bisect points to
>
> commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
> Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Date: Tue Apr 21 17:13:19 2015 +0300
>
> drm/i915: Use atomic helpers for computing changed flags
>
> as the commit that actually triggers the regression.
>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Testcase: igt/kms_universal_plane
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> My original expectation was that we'd need to be updating the in-flight CRTC
> state rather than the committed state as I wound up doing here. However it
> looks like our legacy modesets aren't yet converted to the point where they do
> state swaps, hence the need to update crtc->state directly.
Actually, it looks like I screwed up my working tree and lost the end of
Ander's patch series. We actually *do* have proper CRTC state swapping
now (when his whole series is present), so updating in-flight state is
the right way to go; I'll send a v2 shortly that fixes that.
Matt
>
> Ander/Maarten, does this look like the right place to have 'active' updated for
> the time being, or should it be moved elsewhere in the modeset pipeline?
>
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c297cdc..be166cf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6128,6 +6128,8 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
>
> drm_plane_helper_disable(crtc->primary);
>
> + crtc->state->active = false;
> +
> /* Update computed state. */
> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> if (!connector->encoder || !connector->encoder->crtc)
> --
> 1.8.5.1
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)
2015-05-07 21:28 ` Matt Roper
@ 2015-05-07 21:31 ` Matt Roper
2015-05-08 7:56 ` Ander Conselvan De Oliveira
2015-05-09 5:08 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) shuang.he
0 siblings, 2 replies; 9+ messages in thread
From: Matt Roper @ 2015-05-07 21:31 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
With the recent modeset internal rework, we wind up setting
crtc_state->enable to false, but leave crtc_state->active as true, which
is incorrect. This mismatch gets caught by drm_atomic_crtc_check() and
causes subsequent atomic operations (such as plane updates while the
CRTC is disabled) to fail.
Bisect points to
commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Date: Tue Apr 21 17:13:19 2015 +0300
drm/i915: Use atomic helpers for computing changed flags
as the commit that actually triggers the regression.
v2: Update to alter in-flight state rather than already-committed state
(first version was accidentally based on a midpoint of Ander's
modeset rework series, before his final patches that add proper
state swapping to the legacy modeset path).
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Testcase: igt/kms_universal_plane
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c297cdc..981478a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12340,6 +12340,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
continue;
if (!crtc_state->enable) {
+ crtc_state->active = false;
intel_crtc_disable(crtc);
} else if (crtc->state->enable) {
intel_crtc_disable_planes(crtc);
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)
2015-05-07 21:31 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) Matt Roper
@ 2015-05-08 7:56 ` Ander Conselvan De Oliveira
2015-05-08 8:17 ` Daniel Vetter
2015-05-11 8:45 ` [PATCH] drm/i915: Keep crtc_state->active in sync with enable Maarten Lankhorst
2015-05-09 5:08 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) shuang.he
1 sibling, 2 replies; 9+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-05-08 7:56 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Thu, 2015-05-07 at 14:31 -0700, Matt Roper wrote:
> With the recent modeset internal rework, we wind up setting
> crtc_state->enable to false, but leave crtc_state->active as true, which
> is incorrect. This mismatch gets caught by drm_atomic_crtc_check() and
> causes subsequent atomic operations (such as plane updates while the
> CRTC is disabled) to fail.
>
> Bisect points to
>
> commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
> Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Date: Tue Apr 21 17:13:19 2015 +0300
>
> drm/i915: Use atomic helpers for computing changed flags
>
> as the commit that actually triggers the regression.
>
> v2: Update to alter in-flight state rather than already-committed state
> (first version was accidentally based on a midpoint of Ander's
> modeset rework series, before his final patches that add proper
> state swapping to the legacy modeset path).
>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Testcase: igt/kms_universal_plane
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c297cdc..981478a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12340,6 +12340,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
> continue;
>
> if (!crtc_state->enable) {
> + crtc_state->active = false;
I believe a more appropriate fix would be to set crtc_state->active when
we set crtc_state->enable to false in intel_modeset_stage_output_state()
and the HW state read out. But I think Maarten does that in his later
patch series, so as a stopgap solution this is fine.
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> intel_crtc_disable(crtc);
> } else if (crtc->state->enable) {
> intel_crtc_disable_planes(crtc);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)
2015-05-08 7:56 ` Ander Conselvan De Oliveira
@ 2015-05-08 8:17 ` Daniel Vetter
2015-05-11 8:45 ` [PATCH] drm/i915: Keep crtc_state->active in sync with enable Maarten Lankhorst
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-05-08 8:17 UTC (permalink / raw)
To: Ander Conselvan De Oliveira; +Cc: intel-gfx
On Fri, May 08, 2015 at 10:56:03AM +0300, Ander Conselvan De Oliveira wrote:
> On Thu, 2015-05-07 at 14:31 -0700, Matt Roper wrote:
> > With the recent modeset internal rework, we wind up setting
> > crtc_state->enable to false, but leave crtc_state->active as true, which
> > is incorrect. This mismatch gets caught by drm_atomic_crtc_check() and
> > causes subsequent atomic operations (such as plane updates while the
> > CRTC is disabled) to fail.
> >
> > Bisect points to
> >
> > commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
> > Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Date: Tue Apr 21 17:13:19 2015 +0300
> >
> > drm/i915: Use atomic helpers for computing changed flags
> >
> > as the commit that actually triggers the regression.
> >
> > v2: Update to alter in-flight state rather than already-committed state
> > (first version was accidentally based on a midpoint of Ander's
> > modeset rework series, before his final patches that add proper
> > state swapping to the legacy modeset path).
> >
> > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Testcase: igt/kms_universal_plane
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c297cdc..981478a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12340,6 +12340,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
> > continue;
> >
> > if (!crtc_state->enable) {
> > + crtc_state->active = false;
>
> I believe a more appropriate fix would be to set crtc_state->active when
> we set crtc_state->enable to false in intel_modeset_stage_output_state()
> and the HW state read out. But I think Maarten does that in his later
> patch series, so as a stopgap solution this is fine.
>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)
2015-05-07 21:31 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) Matt Roper
2015-05-08 7:56 ` Ander Conselvan De Oliveira
@ 2015-05-09 5:08 ` shuang.he
1 sibling, 0 replies; 9+ messages in thread
From: shuang.he @ 2015-05-09 5:08 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, matthew.d.roper
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6352
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB -1 314/314 313/314
IVB 338/338 338/338
BYT 286/286 286/286
BDW 320/320 320/320
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
SNB igt@pm_rpm@dpms-mode-unset-non-lpsp DMESG_WARN(2)PASS(1) DMESG_WARN(2)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled
2015-05-07 21:19 [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled Matt Roper
2015-05-07 21:28 ` Matt Roper
@ 2015-05-11 6:11 ` shuang.he
1 sibling, 0 replies; 9+ messages in thread
From: shuang.he @ 2015-05-11 6:11 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, matthew.d.roper
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6351
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 316/316 316/316
IVB 342/342 342/342
BYT 286/286 286/286
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm/i915: Keep crtc_state->active in sync with enable.
2015-05-08 7:56 ` Ander Conselvan De Oliveira
2015-05-08 8:17 ` Daniel Vetter
@ 2015-05-11 8:45 ` Maarten Lankhorst
2015-05-11 9:58 ` Daniel Vetter
1 sibling, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2015-05-11 8:45 UTC (permalink / raw)
To: Ander Conselvan De Oliveira, Matt Roper; +Cc: intel-gfx
With the recent modeset internal rework, we wind up setting crtc_state->enable
to false, but leave crtc_state->active as true following a
drmModeSetCrtc(fb=0), which is incorrect. This mismatch gets caught by
drm_atomic_crtc_check() and causes subsequent atomic operations (such as plane
updates while the CRTC is disabled) to fail.
Bisect points to
commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Date: Tue Apr 21 17:13:19 2015 +0300
drm/i915: Use atomic helpers for computing changed flags
as the commit that actually triggers the regression.
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reported-and-Tested-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
The original patch for this is wrong, we should keep enable and active in sync when changed.
Please revert "drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)".
drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 22e6644f755e..e223209e2d81 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9868,7 +9868,7 @@ retry:
goto fail;
}
- crtc_state->base.enable = true;
+ crtc_state->base.active = crtc_state->base.enable = true;
if (!mode)
mode = &load_detect_mode;
@@ -9965,7 +9965,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
connector_state->best_encoder = NULL;
connector_state->crtc = NULL;
- crtc_state->base.enable = false;
+ crtc_state->base.enable = crtc_state->base.active = false;
ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL,
0, 0);
@@ -12492,7 +12492,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
continue;
}
- crtc_state->base.enable = intel_crtc->new_enabled;
+ crtc_state->base.active = crtc_state->base.enable =
+ intel_crtc->new_enabled;
if (&intel_crtc->base == crtc)
drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
@@ -12617,11 +12618,16 @@ intel_modeset_stage_output_state(struct drm_device *dev,
}
for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ bool has_connectors;
+
ret = drm_atomic_add_affected_connectors(state, crtc);
if (ret)
return ret;
- crtc_state->enable = drm_atomic_connectors_for_crtc(state, crtc);
+ has_connectors = !!drm_atomic_connectors_for_crtc(state, crtc);
+ if (has_connectors != crtc_state->enable)
+ crtc_state->enable =
+ crtc_state->active = has_connectors;
}
ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,
@@ -14598,6 +14604,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
WARN_ON(crtc->active);
crtc->base.state->enable = false;
+ crtc->base.state->active = false;
crtc->base.enabled = false;
}
@@ -14626,6 +14633,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
crtc->active ? "enabled" : "disabled");
crtc->base.state->enable = crtc->active;
+ crtc->base.state->active = crtc->active;
crtc->base.enabled = crtc->active;
/* Because we only establish the connector -> encoder ->
@@ -14764,6 +14772,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
crtc->config);
crtc->base.state->enable = crtc->active;
+ crtc->base.state->active = crtc->active;
crtc->base.enabled = crtc->active;
plane_state = to_intel_plane_state(primary->state);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Keep crtc_state->active in sync with enable.
2015-05-11 8:45 ` [PATCH] drm/i915: Keep crtc_state->active in sync with enable Maarten Lankhorst
@ 2015-05-11 9:58 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2015-05-11 9:58 UTC (permalink / raw)
To: Maarten Lankhorst; +Cc: intel-gfx
On Mon, May 11, 2015 at 10:45:15AM +0200, Maarten Lankhorst wrote:
> With the recent modeset internal rework, we wind up setting crtc_state->enable
> to false, but leave crtc_state->active as true following a
> drmModeSetCrtc(fb=0), which is incorrect. This mismatch gets caught by
> drm_atomic_crtc_check() and causes subsequent atomic operations (such as plane
> updates while the CRTC is disabled) to fail.
>
> Bisect points to
>
> commit dad9a7d6d96630182fb52aae7c3856e9e7285e13
> Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Date: Tue Apr 21 17:13:19 2015 +0300
>
> drm/i915: Use atomic helpers for computing changed flags
>
> as the commit that actually triggers the regression.
>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Reported-and-Tested-by: Kenneth Graunke <kenneth@whitecape.org>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Yeah this makes more sense, in the legacy codepaths we always implicitly
smash the dpms state (i.e. state->active) to the logical pipe state (i.e.
state->enable). Queued for -next, thanks for the patch.
> ---
> The original patch for this is wrong, we should keep enable and active in sync when changed.
> Please revert "drm/i915: Set crtc_state->active to false when CRTC is disabled (v2)".
I've squashed the revert right into this one, imo didn't make sense
separetely.
-Daniel
>
> drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 22e6644f755e..e223209e2d81 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9868,7 +9868,7 @@ retry:
> goto fail;
> }
>
> - crtc_state->base.enable = true;
> + crtc_state->base.active = crtc_state->base.enable = true;
>
> if (!mode)
> mode = &load_detect_mode;
> @@ -9965,7 +9965,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> connector_state->best_encoder = NULL;
> connector_state->crtc = NULL;
>
> - crtc_state->base.enable = false;
> + crtc_state->base.enable = crtc_state->base.active = false;
>
> ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL,
> 0, 0);
> @@ -12492,7 +12492,8 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
> continue;
> }
>
> - crtc_state->base.enable = intel_crtc->new_enabled;
> + crtc_state->base.active = crtc_state->base.enable =
> + intel_crtc->new_enabled;
>
> if (&intel_crtc->base == crtc)
> drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
> @@ -12617,11 +12618,16 @@ intel_modeset_stage_output_state(struct drm_device *dev,
> }
>
> for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + bool has_connectors;
> +
> ret = drm_atomic_add_affected_connectors(state, crtc);
> if (ret)
> return ret;
>
> - crtc_state->enable = drm_atomic_connectors_for_crtc(state, crtc);
> + has_connectors = !!drm_atomic_connectors_for_crtc(state, crtc);
> + if (has_connectors != crtc_state->enable)
> + crtc_state->enable =
> + crtc_state->active = has_connectors;
> }
>
> ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,
> @@ -14598,6 +14604,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>
> WARN_ON(crtc->active);
> crtc->base.state->enable = false;
> + crtc->base.state->active = false;
> crtc->base.enabled = false;
> }
>
> @@ -14626,6 +14633,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
> crtc->active ? "enabled" : "disabled");
>
> crtc->base.state->enable = crtc->active;
> + crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
>
> /* Because we only establish the connector -> encoder ->
> @@ -14764,6 +14772,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> crtc->config);
>
> crtc->base.state->enable = crtc->active;
> + crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
>
> plane_state = to_intel_plane_state(primary->state);
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-05-11 9:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-07 21:19 [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled Matt Roper
2015-05-07 21:28 ` Matt Roper
2015-05-07 21:31 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) Matt Roper
2015-05-08 7:56 ` Ander Conselvan De Oliveira
2015-05-08 8:17 ` Daniel Vetter
2015-05-11 8:45 ` [PATCH] drm/i915: Keep crtc_state->active in sync with enable Maarten Lankhorst
2015-05-11 9:58 ` Daniel Vetter
2015-05-09 5:08 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled (v2) shuang.he
2015-05-11 6:11 ` [PATCH] drm/i915: Set crtc_state->active to false when CRTC is disabled shuang.he
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox