Re: [PATCH v2 1/4] drm/i915/gen9: Only copy WM results for changed pipes to skl_hw

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Lyude <cpaul@redhat.com>,
	intel-gfx@lists.freedesktop.org,
	Matt Roper <matthew.d.roper@intel.com>
Cc: stable@vger.kernel.org,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Radhakrishna Sripada" <radhakrishna.sripada@intel.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"David Airlie" <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/4] drm/i915/gen9: Only copy WM results for changed pipes to skl_hw
Date: Mon, 25 Jul 2016 15:01:02 +0200	[thread overview]
Message-ID: <8ad1925e-3bad-0d8d-f80d-f8d152c2531f@linux.intel.com> (raw)
In-Reply-To: <1469129020-2680-2-git-send-email-cpaul@redhat.com>

Op 21-07-16 om 21:23 schreef Lyude:
> From: Matt Roper <matthew.d.roper@intel.com>
>
> When we write watermark values to the hardware, those values are stored
> in dev_priv->wm.skl_hw.  However with recent watermark changes, the
> results structure we're copying from only contains valid watermark and
> DDB values for the pipes that are actually changing; the values for
> other pipes remain 0.  Thus a blind copy of the entire skl_wm_values
> structure will clobber the values for unchanged pipes...we need to be
> more selective and only copy over the values for the changing pipes.
>
> This mistake was hidden until recently due to another bug that caused us
> to erroneously re-calculate watermarks for all active pipes rather than
> changing pipes.  Only when that bug was fixed was the impact of this bug
> discovered (e.g., modesets failing with "Requested display configuration
> exceeds system watermark limitations" messages and leaving watermarks
> non-functional, even ones initiated by intel_fbdev_restore_mode).
>
> Changes since v1:
>  - Add a function for copying a pipe's wm values
>    (skl_copy_wm_for_pipe()) so we can reuse this later
Looks like I can hit this when I wrote some tests for patch 2 in this series.

testcase will be kms_cursor_legacy.2x-flip-vs-cursor-legacy, but I haven't committed the changes yet.