| Message ID | 1442223011-15581-2-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 14, 2015 at 11:30:11AM +0200, Maarten Lankhorst wrote: > A fast modeset can only be performed when connectors and active are > not changed. This prevents a lot of KMS spam when going from a NULL > mode with 0 connectors to an actual mode. > > When a crtc is inactive there's no need to evade either, the changes > can be applied when the crtc turns on again. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Needs an igt imo. -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index deb76c84a307..eddc81c2d459 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12281,7 +12281,7 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n, > > static bool > intel_pipe_config_compare(struct drm_device *dev, > - struct intel_crtc_state *current_config, > + const struct intel_crtc_state *current_config, > struct intel_crtc_state *pipe_config, > bool adjust) > { > @@ -13013,14 +13013,15 @@ static int intel_atomic_check(struct drm_device *dev, > if (ret) > return ret; > > - if (intel_pipe_config_compare(state->dev, > - to_intel_crtc_state(crtc->state), > - pipe_config, true)) { > + if (!crtc_state->connectors_changed && > + !crtc_state->active_changed && > + crtc_state->active && > + intel_pipe_config_compare(state->dev, > + to_intel_crtc_state(crtc->state), > + pipe_config, true)) { > crtc_state->mode_changed = false; > - to_intel_crtc_state(crtc_state)->update_pipe = true; > - } > - > - if (needs_modeset(crtc_state)) { > + pipe_config->update_pipe = true; > + } else { > any_ms = true; > > ret = drm_atomic_add_affected_planes(state, crtc); > @@ -13029,8 +13030,8 @@ static int intel_atomic_check(struct drm_device *dev, > } > > intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, > - needs_modeset(crtc_state) ? > - "[modeset]" : "[fastset]"); > + pipe_config->update_pipe ? > + "[fastset]" : "[modeset]"); > } > > if (any_ms) { > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 14 September 2015 at 10:30, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > @@ -13013,14 +13013,15 @@ static int intel_atomic_check(struct drm_device *dev, > if (ret) > return ret; > > - if (intel_pipe_config_compare(state->dev, > - to_intel_crtc_state(crtc->state), > - pipe_config, true)) { > + if (!crtc_state->connectors_changed && > + !crtc_state->active_changed && look > + crtc_state->active && > + intel_pipe_config_compare(state->dev, > + to_intel_crtc_state(crtc->state), > + pipe_config, true)) { > crtc_state->mode_changed = false; > - to_intel_crtc_state(crtc_state)->update_pipe = true; > - } > - > - if (needs_modeset(crtc_state)) { > + pipe_config->update_pipe = true; > + } else { > any_ms = true; The change from only setting any_ms if needs_modeset() is true, to always if we can't do a fastset, seems correct but maybe a bit subtle. Was that intended? At the moment it does look like it'll widen the net a little bit, but I _suspect_ that's a good thing. Pending igt: Acked-by: Daniel Stone <daniels@collabora.com> Cheers, Daniel
On 09/14/2015 02:30 AM, Maarten Lankhorst wrote: > + if (!crtc_state->connectors_changed && > + !crtc_state->active_changed && > + crtc_state->active && > + intel_pipe_config_compare(state->dev, > + to_intel_crtc_state(crtc->state), > + pipe_config, true)) { > crtc_state->mode_changed = false; > - to_intel_crtc_state(crtc_state)->update_pipe = true; > - } Doesn't needs_modeset() cover the connectors and active changed cases? Could we just hoist that up earlier in this function instead? Jesse
Op 16-09-15 om 20:48 schreef Jesse Barnes: > On 09/14/2015 02:30 AM, Maarten Lankhorst wrote: >> + if (!crtc_state->connectors_changed && >> + !crtc_state->active_changed && >> + crtc_state->active && >> + intel_pipe_config_compare(state->dev, >> + to_intel_crtc_state(crtc->state), >> + pipe_config, true)) { >> crtc_state->mode_changed = false; >> - to_intel_crtc_state(crtc_state)->update_pipe = true; >> - } > Doesn't needs_modeset() cover the connectors and active changed cases? Could we just hoist that up earlier in this function instead? > Not sure what you're saying, since needs_modeset is always true here. ~Maarten
Op 14-09-15 om 19:10 schreef Daniel Stone: > Hi, > > On 14 September 2015 at 10:30, Maarten Lankhorst > <maarten.lankhorst@linux.intel.com> wrote: >> @@ -13013,14 +13013,15 @@ static int intel_atomic_check(struct drm_device *dev, >> if (ret) >> return ret; >> >> - if (intel_pipe_config_compare(state->dev, >> - to_intel_crtc_state(crtc->state), >> - pipe_config, true)) { >> + if (!crtc_state->connectors_changed && >> + !crtc_state->active_changed && look >> + crtc_state->active && >> + intel_pipe_config_compare(state->dev, >> + to_intel_crtc_state(crtc->state), >> + pipe_config, true)) { >> crtc_state->mode_changed = false; >> - to_intel_crtc_state(crtc_state)->update_pipe = true; >> - } >> - >> - if (needs_modeset(crtc_state)) { >> + pipe_config->update_pipe = true; >> + } else { >> any_ms = true; > The change from only setting any_ms if needs_modeset() is true, to > always if we can't do a fastset, seems correct but maybe a bit subtle. It's exactly the same thing, just made a bit more explicit. before: any_ms = needs_modeset() with mode_changed = !update_pipe. After: any_ms = !update_pipe. > Was that intended? At the moment it does look like it'll widen the net > a little bit, but I _suspect_ that's a good thing. Pending igt: > Acked-by: Daniel Stone <daniels@collabora.com> > > Cheers, > Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index deb76c84a307..eddc81c2d459 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12281,7 +12281,7 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n, static bool intel_pipe_config_compare(struct drm_device *dev, - struct intel_crtc_state *current_config, + const struct intel_crtc_state *current_config, struct intel_crtc_state *pipe_config, bool adjust) { @@ -13013,14 +13013,15 @@ static int intel_atomic_check(struct drm_device *dev, if (ret) return ret; - if (intel_pipe_config_compare(state->dev, - to_intel_crtc_state(crtc->state), - pipe_config, true)) { + if (!crtc_state->connectors_changed && + !crtc_state->active_changed && + crtc_state->active && + intel_pipe_config_compare(state->dev, + to_intel_crtc_state(crtc->state), + pipe_config, true)) { crtc_state->mode_changed = false; - to_intel_crtc_state(crtc_state)->update_pipe = true; - } - - if (needs_modeset(crtc_state)) { + pipe_config->update_pipe = true; + } else { any_ms = true; ret = drm_atomic_add_affected_planes(state, crtc); @@ -13029,8 +13030,8 @@ static int intel_atomic_check(struct drm_device *dev, } intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, - needs_modeset(crtc_state) ? - "[modeset]" : "[fastset]"); + pipe_config->update_pipe ? + "[fastset]" : "[modeset]"); } if (any_ms) {
A fast modeset can only be performed when connectors and active are not changed. This prevents a lot of KMS spam when going from a NULL mode with 0 connectors to an actual mode. When a crtc is inactive there's no need to evade either, the changes can be applied when the crtc turns on again. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)