| Message ID | 1431354318-11995-33-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 11, 2015 at 04:25:08PM +0200, Maarten Lankhorst wrote: > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++---------- > drivers/gpu/drm/i915/intel_drv.h | 2 + > 2 files changed, 81 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7bc78b49f9f4..7a79659dca00 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4848,35 +4848,6 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) > return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; > } > > -/* > - * This implements the workaround described in the "notes" section of the mode > - * set sequence documentation. When going from no pipes or single pipe to > - * multiple pipes, and planes are enabled after the pipe, we need to wait at > - * least 2 vblanks on the first pipe before enabling planes on the second pipe. > - */ > -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) > -{ > - struct drm_device *dev = crtc->base.dev; > - struct intel_crtc *crtc_it, *other_active_crtc = NULL; > - > - /* We want to get the other_active_crtc only if there's only 1 other > - * active crtc. */ > - for_each_intel_crtc(dev, crtc_it) { > - if (!crtc_it->active || crtc_it == crtc) > - continue; > - > - if (other_active_crtc) > - return; > - > - other_active_crtc = crtc_it; > - } > - if (!other_active_crtc) > - return; > - > - intel_wait_for_vblank(dev, other_active_crtc->pipe); > - intel_wait_for_vblank(dev, other_active_crtc->pipe); > -} > - > static void haswell_crtc_enable(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > @@ -4967,7 +4938,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > > /* If we change the relative order between pipe/planes enabling, we need > * to change the workaround. */ > - haswell_mode_set_planes_workaround(intel_crtc); > + if (pipe_config->hsw_workaround_pipe != INVALID_PIPE) { > + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); > + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); > + } > } > > static void ironlake_pfit_disable(struct intel_crtc *crtc) > @@ -12147,6 +12121,74 @@ done: > return ret; > } > > +/* > + * This implements the workaround described in the "notes" section of the mode > + * set sequence documentation. When going from no pipes or single pipe to > + * multiple pipes, and planes are enabled after the pipe, we need to wait at > + * least 2 vblanks on the first pipe before enabling planes on the second pipe. > + */ > +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) > +{ > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + struct intel_crtc_state *first_crtc_state = NULL, *other_crtc_state = NULL; > + struct intel_crtc *intel_crtc, *first_crtc = NULL, *enabled_crtc; > + int enabled_crtcs = 0, ret, i; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + struct intel_crtc_state *pipe_config = > + to_intel_crtc_state(crtc_state); > + > + pipe_config->hsw_workaround_pipe = INVALID_PIPE; This kind of state resetting should be done in duplicate_state. > + > + if (!crtc_state->active) > + continue; > + > + if (!needs_modeset(crtc_state)) { > + enabled_crtcs++; > + enabled_crtc = to_intel_crtc(crtc); > + continue; > + } > + > + if (first_crtc) { > + other_crtc_state = pipe_config; > + break; > + } > + first_crtc = to_intel_crtc(crtc); > + first_crtc_state = pipe_config; > + } > + > + /* No workaround needed? */ > + if (!first_crtc || enabled_crtcs > 1) > + return 0; > + > + for_each_intel_crtc(state->dev, intel_crtc) { > + if (state->crtcs[drm_crtc_index(&intel_crtc->base)]) > + continue; > + > + ret = drm_modeset_lock(&intel_crtc->base.mutex, > + state->acquire_ctx); > + if (ret) > + return ret; > + > + if (!intel_crtc->base.state->active) > + continue; Imo just unconditionally acquire the crtc state, there' shouldn't be any harm in that (as long as we leave mode/active/planes_changed untouched). > + > + /* 2 enabled crtcs means no need for w/a */ > + if (++enabled_crtcs >= 2) > + return 0; > + > + enabled_crtc = intel_crtc; > + } > + > + if (enabled_crtcs == 1) > + first_crtc_state->hsw_workaround_pipe = enabled_crtc->pipe; first_crtc_state could be miscomputed if 1 crtc is already on and you add another one. Then first_crtc will point at the other crtc since that's the only one at first in the atomic set. Imo a simpler algo would be: 1. Check whether your activating any crtc at all. 2. If so add all crtc states. 3. Same loop as the old one, just using for_each_crtc_in_state. There's other cases with shared resources where we need to grab all crtc locks already anyway (shared dpll), trying to be clever doesn't seem beneficial. And if this indeed becomes a problem then we need a global state ww mutex and use that (instead of crtc locks) for this book-keeping: First enable crtc would set global_state->hsw_wa_pipe, 2nd and later would clear it. We might need this eventually (I've heard rumours about people not liking stalls when unplugging external screens that much), but let's not overcomplicate things while we do this conversion. -Daniel > + else if (other_crtc_state) > + other_crtc_state->hsw_workaround_pipe = first_crtc->pipe; > + > + return 0; > +} > + > /* Code that should eventually be part of atomic_check() */ > static int __intel_set_mode_checks(struct drm_atomic_state *state) > { > @@ -12158,6 +12200,13 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) > return -EINVAL; > } > > + if (IS_HASWELL(state->dev)) { > + ret = haswell_mode_set_planes_workaround(state); > + > + if (ret) > + return ret; > + } > + > /* > * See if the config requires any additional preparation, e.g. > * to adjust global state with pipes off. We need to do this > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index eb87f82b5aff..6c2ba5dbcd79 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -437,6 +437,8 @@ struct intel_crtc_state { > int pbn; > > struct intel_crtc_scaler_state scaler_state; > + > + enum pipe hsw_workaround_pipe; > }; > > struct intel_pipe_wm { > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 12-05-15 om 11:43 schreef Daniel Vetter: > On Mon, May 11, 2015 at 04:25:08PM +0200, Maarten Lankhorst wrote: >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++---------- >> drivers/gpu/drm/i915/intel_drv.h | 2 + >> 2 files changed, 81 insertions(+), 30 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 7bc78b49f9f4..7a79659dca00 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4848,35 +4848,6 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) >> return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; >> } >> >> -/* >> - * This implements the workaround described in the "notes" section of the mode >> - * set sequence documentation. When going from no pipes or single pipe to >> - * multiple pipes, and planes are enabled after the pipe, we need to wait at >> - * least 2 vblanks on the first pipe before enabling planes on the second pipe. >> - */ >> -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) >> -{ >> - struct drm_device *dev = crtc->base.dev; >> - struct intel_crtc *crtc_it, *other_active_crtc = NULL; >> - >> - /* We want to get the other_active_crtc only if there's only 1 other >> - * active crtc. */ >> - for_each_intel_crtc(dev, crtc_it) { >> - if (!crtc_it->active || crtc_it == crtc) >> - continue; >> - >> - if (other_active_crtc) >> - return; >> - >> - other_active_crtc = crtc_it; >> - } >> - if (!other_active_crtc) >> - return; >> - >> - intel_wait_for_vblank(dev, other_active_crtc->pipe); >> - intel_wait_for_vblank(dev, other_active_crtc->pipe); >> -} >> - >> static void haswell_crtc_enable(struct drm_crtc *crtc) >> { >> struct drm_device *dev = crtc->dev; >> @@ -4967,7 +4938,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) >> >> /* If we change the relative order between pipe/planes enabling, we need >> * to change the workaround. */ >> - haswell_mode_set_planes_workaround(intel_crtc); >> + if (pipe_config->hsw_workaround_pipe != INVALID_PIPE) { >> + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); >> + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); >> + } >> } >> >> static void ironlake_pfit_disable(struct intel_crtc *crtc) >> @@ -12147,6 +12121,74 @@ done: >> return ret; >> } >> >> +/* >> + * This implements the workaround described in the "notes" section of the mode >> + * set sequence documentation. When going from no pipes or single pipe to >> + * multiple pipes, and planes are enabled after the pipe, we need to wait at >> + * least 2 vblanks on the first pipe before enabling planes on the second pipe. >> + */ >> +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) >> +{ >> + struct drm_crtc *crtc; >> + struct drm_crtc_state *crtc_state; >> + struct intel_crtc_state *first_crtc_state = NULL, *other_crtc_state = NULL; >> + struct intel_crtc *intel_crtc, *first_crtc = NULL, *enabled_crtc; >> + int enabled_crtcs = 0, ret, i; >> + >> + for_each_crtc_in_state(state, crtc, crtc_state, i) { >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc_state); >> + >> + pipe_config->hsw_workaround_pipe = INVALID_PIPE; > This kind of state resetting should be done in duplicate_state. The v2 of this patch uses intel_crtc->atomic.hsw_workaround pipe, which would work better. :) >> + >> + if (!crtc_state->active) >> + continue; >> + >> + if (!needs_modeset(crtc_state)) { >> + enabled_crtcs++; >> + enabled_crtc = to_intel_crtc(crtc); >> + continue; >> + } >> + >> + if (first_crtc) { >> + other_crtc_state = pipe_config; >> + break; >> + } >> + first_crtc = to_intel_crtc(crtc); >> + first_crtc_state = pipe_config; >> + } >> + >> + /* No workaround needed? */ >> + if (!first_crtc || enabled_crtcs > 1) >> + return 0; >> + >> + for_each_intel_crtc(state->dev, intel_crtc) { >> + if (state->crtcs[drm_crtc_index(&intel_crtc->base)]) >> + continue; >> + >> + ret = drm_modeset_lock(&intel_crtc->base.mutex, >> + state->acquire_ctx); >> + if (ret) >> + return ret; >> + >> + if (!intel_crtc->base.state->active) >> + continue; > Imo just unconditionally acquire the crtc state, there' shouldn't be any > harm in that (as long as we leave mode/active/planes_changed untouched). Probably. >> + >> + /* 2 enabled crtcs means no need for w/a */ >> + if (++enabled_crtcs >= 2) >> + return 0; >> + >> + enabled_crtc = intel_crtc; >> + } >> + >> + if (enabled_crtcs == 1) >> + first_crtc_state->hsw_workaround_pipe = enabled_crtc->pipe; > first_crtc_state could be miscomputed if 1 crtc is already on and you add > another one. Then first_crtc will point at the other crtc since that's the > only one at first in the atomic set. Imo a simpler algo would be: No, in that case enabled_crtcs != 0 and enabled_crtc is set to the correct crtc. > 1. Check whether your activating any crtc at all. > 2. If so add all crtc states. > 3. Same loop as the old one, just using for_each_crtc_in_state. > > There's other cases with shared resources where we need to grab all crtc > locks already anyway (shared dpll), trying to be clever doesn't seem > beneficial. And if this indeed becomes a problem then we need a global > state ww mutex and use that (instead of crtc locks) for this book-keeping: > First enable crtc would set global_state->hsw_wa_pipe, 2nd and later would > clear it. We might need this eventually (I've heard rumours about people > not liking stalls when unplugging external screens that much), but let's > not overcomplicate things while we do this conversion. You would still need it when disabling crtc's then. To do it right you would need to keep crtc->active, or use a crtc_mask, but that would be abused so I'd rather not. The crtc lock is also needed so the other crtc's cannot be disabled or enabled while modesetting, but I guess always taking the connector_mutex for that could work too. ~Maarten
On Tue, May 12, 2015 at 04:05:24PM +0200, Maarten Lankhorst wrote: > Op 12-05-15 om 11:43 schreef Daniel Vetter: > > On Mon, May 11, 2015 at 04:25:08PM +0200, Maarten Lankhorst wrote: > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++---------- > >> drivers/gpu/drm/i915/intel_drv.h | 2 + > >> 2 files changed, 81 insertions(+), 30 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index 7bc78b49f9f4..7a79659dca00 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -4848,35 +4848,6 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) > >> return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; > >> } > >> > >> -/* > >> - * This implements the workaround described in the "notes" section of the mode > >> - * set sequence documentation. When going from no pipes or single pipe to > >> - * multiple pipes, and planes are enabled after the pipe, we need to wait at > >> - * least 2 vblanks on the first pipe before enabling planes on the second pipe. > >> - */ > >> -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) > >> -{ > >> - struct drm_device *dev = crtc->base.dev; > >> - struct intel_crtc *crtc_it, *other_active_crtc = NULL; > >> - > >> - /* We want to get the other_active_crtc only if there's only 1 other > >> - * active crtc. */ > >> - for_each_intel_crtc(dev, crtc_it) { > >> - if (!crtc_it->active || crtc_it == crtc) > >> - continue; > >> - > >> - if (other_active_crtc) > >> - return; > >> - > >> - other_active_crtc = crtc_it; > >> - } > >> - if (!other_active_crtc) > >> - return; > >> - > >> - intel_wait_for_vblank(dev, other_active_crtc->pipe); > >> - intel_wait_for_vblank(dev, other_active_crtc->pipe); > >> -} > >> - > >> static void haswell_crtc_enable(struct drm_crtc *crtc) > >> { > >> struct drm_device *dev = crtc->dev; > >> @@ -4967,7 +4938,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > >> > >> /* If we change the relative order between pipe/planes enabling, we need > >> * to change the workaround. */ > >> - haswell_mode_set_planes_workaround(intel_crtc); > >> + if (pipe_config->hsw_workaround_pipe != INVALID_PIPE) { > >> + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); > >> + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); > >> + } > >> } > >> > >> static void ironlake_pfit_disable(struct intel_crtc *crtc) > >> @@ -12147,6 +12121,74 @@ done: > >> return ret; > >> } > >> > >> +/* > >> + * This implements the workaround described in the "notes" section of the mode > >> + * set sequence documentation. When going from no pipes or single pipe to > >> + * multiple pipes, and planes are enabled after the pipe, we need to wait at > >> + * least 2 vblanks on the first pipe before enabling planes on the second pipe. > >> + */ > >> +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) > >> +{ > >> + struct drm_crtc *crtc; > >> + struct drm_crtc_state *crtc_state; > >> + struct intel_crtc_state *first_crtc_state = NULL, *other_crtc_state = NULL; > >> + struct intel_crtc *intel_crtc, *first_crtc = NULL, *enabled_crtc; > >> + int enabled_crtcs = 0, ret, i; > >> + > >> + for_each_crtc_in_state(state, crtc, crtc_state, i) { > >> + struct intel_crtc_state *pipe_config = > >> + to_intel_crtc_state(crtc_state); > >> + > >> + pipe_config->hsw_workaround_pipe = INVALID_PIPE; > > This kind of state resetting should be done in duplicate_state. > The v2 of this patch uses intel_crtc->atomic.hsw_workaround pipe, which would work better. :) > >> + > >> + if (!crtc_state->active) > >> + continue; > >> + > >> + if (!needs_modeset(crtc_state)) { > >> + enabled_crtcs++; > >> + enabled_crtc = to_intel_crtc(crtc); > >> + continue; > >> + } > >> + > >> + if (first_crtc) { > >> + other_crtc_state = pipe_config; > >> + break; > >> + } > >> + first_crtc = to_intel_crtc(crtc); > >> + first_crtc_state = pipe_config; > >> + } > >> + > >> + /* No workaround needed? */ > >> + if (!first_crtc || enabled_crtcs > 1) > >> + return 0; > >> + > >> + for_each_intel_crtc(state->dev, intel_crtc) { > >> + if (state->crtcs[drm_crtc_index(&intel_crtc->base)]) > >> + continue; > >> + > >> + ret = drm_modeset_lock(&intel_crtc->base.mutex, > >> + state->acquire_ctx); > >> + if (ret) > >> + return ret; > >> + > >> + if (!intel_crtc->base.state->active) > >> + continue; > > Imo just unconditionally acquire the crtc state, there' shouldn't be any > > harm in that (as long as we leave mode/active/planes_changed untouched). > Probably. > >> + > >> + /* 2 enabled crtcs means no need for w/a */ > >> + if (++enabled_crtcs >= 2) > >> + return 0; > >> + > >> + enabled_crtc = intel_crtc; > >> + } > >> + > >> + if (enabled_crtcs == 1) > >> + first_crtc_state->hsw_workaround_pipe = enabled_crtc->pipe; > > first_crtc_state could be miscomputed if 1 crtc is already on and you add > > another one. Then first_crtc will point at the other crtc since that's the > > only one at first in the atomic set. Imo a simpler algo would be: > No, in that case enabled_crtcs != 0 and enabled_crtc is set to the correct crtc. Hm right I mixed up first_crtc_state and enabled_crtc. Looks correct on second pass. > > 1. Check whether your activating any crtc at all. > > 2. If so add all crtc states. > > 3. Same loop as the old one, just using for_each_crtc_in_state. > > > > There's other cases with shared resources where we need to grab all crtc > > locks already anyway (shared dpll), trying to be clever doesn't seem > > beneficial. And if this indeed becomes a problem then we need a global > > state ww mutex and use that (instead of crtc locks) for this book-keeping: > > First enable crtc would set global_state->hsw_wa_pipe, 2nd and later would > > clear it. We might need this eventually (I've heard rumours about people > > not liking stalls when unplugging external screens that much), but let's > > not overcomplicate things while we do this conversion. > You would still need it when disabling crtc's then. To do it right you would need to keep crtc->active, > or use a crtc_mask, but that would be abused so I'd rather not. The crtc lock is also needed so the > other crtc's cannot be disabled or enabled while modesetting, but I guess always taking the > connector_mutex for that could work too. Hm yeah connector_mutex is the general "global modeset state" lock. I guess we could instead acquire that one and make sure that crtc_state->active is also protected by it. I.e. have a for_each_crtc_in_state(crtc_state) if (crtc_state->active_changed) acquire_connection_mutex(); Maybe even do this in the helper check_modeset function? Well looking at that one it's kinda guaranteed already, so a WARN_ON(!ww_mutex_locked) should do the trick too. Plus a comment that crtc_state->active is protected by the connection_mutex thanks to the helper's logic. Going with that is fine with me too, I just feel like the current open-coded locking logic is a bit too clever/complicated for what we want to do. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7bc78b49f9f4..7a79659dca00 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4848,35 +4848,6 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; } -/* - * This implements the workaround described in the "notes" section of the mode - * set sequence documentation. When going from no pipes or single pipe to - * multiple pipes, and planes are enabled after the pipe, we need to wait at - * least 2 vblanks on the first pipe before enabling planes on the second pipe. - */ -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) -{ - struct drm_device *dev = crtc->base.dev; - struct intel_crtc *crtc_it, *other_active_crtc = NULL; - - /* We want to get the other_active_crtc only if there's only 1 other - * active crtc. */ - for_each_intel_crtc(dev, crtc_it) { - if (!crtc_it->active || crtc_it == crtc) - continue; - - if (other_active_crtc) - return; - - other_active_crtc = crtc_it; - } - if (!other_active_crtc) - return; - - intel_wait_for_vblank(dev, other_active_crtc->pipe); - intel_wait_for_vblank(dev, other_active_crtc->pipe); -} - static void haswell_crtc_enable(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -4967,7 +4938,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) /* If we change the relative order between pipe/planes enabling, we need * to change the workaround. */ - haswell_mode_set_planes_workaround(intel_crtc); + if (pipe_config->hsw_workaround_pipe != INVALID_PIPE) { + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); + intel_wait_for_vblank(dev, pipe_config->hsw_workaround_pipe); + } } static void ironlake_pfit_disable(struct intel_crtc *crtc) @@ -12147,6 +12121,74 @@ done: return ret; } +/* + * This implements the workaround described in the "notes" section of the mode + * set sequence documentation. When going from no pipes or single pipe to + * multiple pipes, and planes are enabled after the pipe, we need to wait at + * least 2 vblanks on the first pipe before enabling planes on the second pipe. + */ +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct intel_crtc_state *first_crtc_state = NULL, *other_crtc_state = NULL; + struct intel_crtc *intel_crtc, *first_crtc = NULL, *enabled_crtc; + int enabled_crtcs = 0, ret, i; + + for_each_crtc_in_state(state, crtc, crtc_state, i) { + struct intel_crtc_state *pipe_config = + to_intel_crtc_state(crtc_state); + + pipe_config->hsw_workaround_pipe = INVALID_PIPE; + + if (!crtc_state->active) + continue; + + if (!needs_modeset(crtc_state)) { + enabled_crtcs++; + enabled_crtc = to_intel_crtc(crtc); + continue; + } + + if (first_crtc) { + other_crtc_state = pipe_config; + break; + } + first_crtc = to_intel_crtc(crtc); + first_crtc_state = pipe_config; + } + + /* No workaround needed? */ + if (!first_crtc || enabled_crtcs > 1) + return 0; + + for_each_intel_crtc(state->dev, intel_crtc) { + if (state->crtcs[drm_crtc_index(&intel_crtc->base)]) + continue; + + ret = drm_modeset_lock(&intel_crtc->base.mutex, + state->acquire_ctx); + if (ret) + return ret; + + if (!intel_crtc->base.state->active) + continue; + + /* 2 enabled crtcs means no need for w/a */ + if (++enabled_crtcs >= 2) + return 0; + + enabled_crtc = intel_crtc; + } + + if (enabled_crtcs == 1) + first_crtc_state->hsw_workaround_pipe = enabled_crtc->pipe; + else if (other_crtc_state) + other_crtc_state->hsw_workaround_pipe = first_crtc->pipe; + + return 0; +} + /* Code that should eventually be part of atomic_check() */ static int __intel_set_mode_checks(struct drm_atomic_state *state) { @@ -12158,6 +12200,13 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) return -EINVAL; } + if (IS_HASWELL(state->dev)) { + ret = haswell_mode_set_planes_workaround(state); + + if (ret) + return ret; + } + /* * See if the config requires any additional preparation, e.g. * to adjust global state with pipes off. We need to do this diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index eb87f82b5aff..6c2ba5dbcd79 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -437,6 +437,8 @@ struct intel_crtc_state { int pbn; struct intel_crtc_scaler_state scaler_state; + + enum pipe hsw_workaround_pipe; }; struct intel_pipe_wm {
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 109 +++++++++++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 2 + 2 files changed, 81 insertions(+), 30 deletions(-)