Re: [PATCH 1/3] drm/i915/gen9: Initialize intel_state->active_crtcs during WM sanitization

Daniel Vetter
2016-06-13T14:49:53Z

On Thu, Jun 09, 2016 at 03:14:53PM -0700, Matt Roper wrote:
> intel_state->active_crtcs is usually only initialized when doing a
> modeset. During our first atomic commit after boot, we're effectively
> faking a modeset to sanitize the DDB/wm setup, so ensure that this field
> gets initialized before use.
>
> Reported-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 658a756..0cd38ca 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3896,9 +3896,18 @@ skl_compute_ddb(struct drm_atomic_state *state)
> * pretend that all pipes switched active status so that we'll
> * ensure a full DDB recompute.
> */
> - if (dev_priv->wm.distrust_bios_wm)
> + if (dev_priv->wm.distrust_bios_wm) {
> intel_state->active_pipe_changes = ~0;
>
> + /*
> + * We usually only initialize intel_state->active_crtcs if we
> + * we're doing a modeset; make sure this field is always
> + * initialized during the sanitization process that happens
> + * on the first commit too.
> + */
> + intel_state->active_crtcs = dev_priv->active_crtcs;
> + }

Can't we move input sanitization out of this? Imo mixing up atomic
check/compute config code with hw state restoring is way too fragile.

Also, why exactly do we have active_crtcs? Seems to just be duplicated
state that can get out of sync, we have too many of those already ...
-Daniel

> +
> /*
> * If the modeset changes which CRTC's are active, we need to
> * recompute the DDB allocation for *all* active pipes, even
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation