* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.