| Message ID | 1458741487-23801-3-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 23, 2016 at 02:58:07PM +0100, Maarten Lankhorst wrote: > The modeset state checker no longer has full access to the hardware, > instead it should only check affected crtc's. > > Looking for disabled stuff can be checked immediately after all crtc > disables have completed, while each enabled crtc can be checked right > after being enabled. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 24 +++++------------------- > 1 file changed, 5 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 4148b262f2a7..b6a75aa32e5a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13027,28 +13027,13 @@ check_disabled_dpll_state(struct drm_device *dev) > } > > static void > -intel_modeset_check_disabled(struct drm_device *dev, > - struct drm_atomic_state *old_state) > +intel_modeset_check_disabled(struct drm_device *dev) > { > check_encoder_state(dev); > check_connector_state(dev, NULL); > check_disabled_dpll_state(dev); > } > > -static void > -intel_modeset_check_state(struct drm_device *dev, > - struct drm_atomic_state *old_state) > -{ > - struct drm_crtc_state *old_crtc_state; > - struct drm_crtc *crtc; > - int i; > - > - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) > - intel_modeset_check_crtc(crtc, old_crtc_state, crtc->state); > - > - intel_modeset_check_disabled(dev, old_state); > -} > - > static void update_scanline_offset(struct intel_crtc *crtc) > { > struct drm_device *dev = crtc->base.dev; > @@ -13616,6 +13601,8 @@ static int intel_atomic_commit(struct drm_device *dev, > if (dev_priv->display.modeset_commit_cdclk && > intel_state->dev_cdclk != dev_priv->cdclk_freq) > dev_priv->display.modeset_commit_cdclk(state); > + > + intel_modeset_check_disabled(dev); As noted on patch #1, since we're checking all disabled state (and not just the things that are newly-disabled by this transaction), we may not hold full locks over everything that's disabled here and might be racing with other updates. But we're no worse off than we were before. As a minor behavior change, it looks like we're also no longer calling this when we're doing non-modeset pipe updates, but that should be fine. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > } > > /* Now enable the clocks, plane, pipe, and connectors that we set up. */ > @@ -13682,6 +13669,8 @@ static int intel_atomic_commit(struct drm_device *dev, > > if (put_domains[i]) > modeset_put_power_domains(dev_priv, put_domains[i]); > + > + intel_modeset_check_crtc(crtc, old_crtc_state, crtc->state); > } > > if (intel_state->modeset) > @@ -13691,9 +13680,6 @@ static int intel_atomic_commit(struct drm_device *dev, > drm_atomic_helper_cleanup_planes(dev, state); > mutex_unlock(&dev->struct_mutex); > > - if (hw_check) > - intel_modeset_check_state(dev, state); > - > drm_atomic_state_free(state); > > /* As one of the primary mmio accessors, KMS has a high likelihood > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 08-04-16 om 01:23 schreef Matt Roper: > On Wed, Mar 23, 2016 at 02:58:07PM +0100, Maarten Lankhorst wrote: >> The modeset state checker no longer has full access to the hardware, >> instead it should only check affected crtc's. >> >> Looking for disabled stuff can be checked immediately after all crtc >> disables have completed, while each enabled crtc can be checked right >> after being enabled. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 24 +++++------------------- >> 1 file changed, 5 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 4148b262f2a7..b6a75aa32e5a 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -13027,28 +13027,13 @@ check_disabled_dpll_state(struct drm_device *dev) >> } >> >> static void >> -intel_modeset_check_disabled(struct drm_device *dev, >> - struct drm_atomic_state *old_state) >> +intel_modeset_check_disabled(struct drm_device *dev) >> { >> check_encoder_state(dev); >> check_connector_state(dev, NULL); >> check_disabled_dpll_state(dev); >> } >> >> -static void >> -intel_modeset_check_state(struct drm_device *dev, >> - struct drm_atomic_state *old_state) >> -{ >> - struct drm_crtc_state *old_crtc_state; >> - struct drm_crtc *crtc; >> - int i; >> - >> - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) >> - intel_modeset_check_crtc(crtc, old_crtc_state, crtc->state); >> - >> - intel_modeset_check_disabled(dev, old_state); >> -} >> - >> static void update_scanline_offset(struct intel_crtc *crtc) >> { >> struct drm_device *dev = crtc->base.dev; >> @@ -13616,6 +13601,8 @@ static int intel_atomic_commit(struct drm_device *dev, >> if (dev_priv->display.modeset_commit_cdclk && >> intel_state->dev_cdclk != dev_priv->cdclk_freq) >> dev_priv->display.modeset_commit_cdclk(state); >> + >> + intel_modeset_check_disabled(dev); > As noted on patch #1, since we're checking all disabled state (and not > just the things that are newly-disabled by this transaction), we may not > hold full locks over everything that's disabled here and might be racing > with other updates. But we're no worse off than we were before. > > As a minor behavior change, it looks like we're also no longer calling > this when we're doing non-modeset pipe updates, but that should be fine. > This is true, but enabling a pipe requires connection_mutex, which is always held if we do a modeset anyway..
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4148b262f2a7..b6a75aa32e5a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13027,28 +13027,13 @@ check_disabled_dpll_state(struct drm_device *dev) } static void -intel_modeset_check_disabled(struct drm_device *dev, - struct drm_atomic_state *old_state) +intel_modeset_check_disabled(struct drm_device *dev) { check_encoder_state(dev); check_connector_state(dev, NULL); check_disabled_dpll_state(dev); } -static void -intel_modeset_check_state(struct drm_device *dev, - struct drm_atomic_state *old_state) -{ - struct drm_crtc_state *old_crtc_state; - struct drm_crtc *crtc; - int i; - - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) - intel_modeset_check_crtc(crtc, old_crtc_state, crtc->state); - - intel_modeset_check_disabled(dev, old_state); -} - static void update_scanline_offset(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -13616,6 +13601,8 @@ static int intel_atomic_commit(struct drm_device *dev, if (dev_priv->display.modeset_commit_cdclk && intel_state->dev_cdclk != dev_priv->cdclk_freq) dev_priv->display.modeset_commit_cdclk(state); + + intel_modeset_check_disabled(dev); } /* Now enable the clocks, plane, pipe, and connectors that we set up. */ @@ -13682,6 +13669,8 @@ static int intel_atomic_commit(struct drm_device *dev, if (put_domains[i]) modeset_put_power_domains(dev_priv, put_domains[i]); + + intel_modeset_check_crtc(crtc, old_crtc_state, crtc->state); } if (intel_state->modeset) @@ -13691,9 +13680,6 @@ static int intel_atomic_commit(struct drm_device *dev, drm_atomic_helper_cleanup_planes(dev, state); mutex_unlock(&dev->struct_mutex); - if (hw_check) - intel_modeset_check_state(dev, state); - drm_atomic_state_free(state); /* As one of the primary mmio accessors, KMS has a high likelihood
The modeset state checker no longer has full access to the hardware, instead it should only check affected crtc's. Looking for disabled stuff can be checked immediately after all crtc disables have completed, while each enabled crtc can be checked right after being enabled. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-)