From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com ([134.134.136.31]:50848 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751159AbdEaK7a (ORCPT ); Wed, 31 May 2017 06:59:30 -0400 Subject: Re: [PATCH] drm/i915: Always recompute watermarks when distrust_bios_wm is set To: Mahesh Kumar , intel-gfx@lists.freedesktop.org Cc: "# v4 . 8+" , Matt Roper References: <20170526151546.25025-2-mahesh1.kumar@intel.com> <20170531075646.15158-1-maarten.lankhorst@linux.intel.com> <4b0803df-ee99-f4a4-1b36-aa1ec3f47ff6@intel.com> From: Maarten Lankhorst Message-ID: <4d4de194-0777-834f-302d-91c3ffb31195@linux.intel.com> Date: Wed, 31 May 2017 12:59:07 +0200 MIME-Version: 1.0 In-Reply-To: <4b0803df-ee99-f4a4-1b36-aa1ec3f47ff6@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: stable-owner@vger.kernel.org List-ID: Op 31-05-17 om 12:42 schreef Mahesh Kumar: > Hi, > > > On Wednesday 31 May 2017 01:26 PM, Maarten Lankhorst wrote: >> On some systems there can be a race condition in which no crtc state is >> added to the first atomic commit. This results in all crtc's having a >> null DDB allocation, causing a FIFO underrun on any update until the >> first modeset. >> >> Reported-by: Maarten Lankhorst >> Inspired-by: Mahesh Kumar >> Signed-off-by: Maarten Lankhorst >> Fixes: 98d39494d375 ("drm/i915/gen9: Compute DDB allocation at atomic >> check time (v4)") >> Cc: # v4.8+ >> Cc: Mahesh Kumar >> Cc: Matt Roper >> --- >> drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index a488e068c3d6..8daa382b47f3 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -5037,10 +5037,27 @@ skl_compute_wm(struct drm_atomic_state *state) >> struct drm_crtc_state *cstate; >> struct intel_atomic_state *intel_state = to_intel_atomic_state(state); >> struct skl_wm_values *results = &intel_state->wm_results; >> + struct drm_device *dev = state->dev; >> struct skl_pipe_wm *pipe_wm; >> bool changed = false; >> int ret, i; >> + if (to_i915(dev)->wm.distrust_bios_wm) { >> + changed = true; >> + >> + /* >> + * The first commit after driver load must update the ddb >> + * allocations, so make sure we at least hold a lock to >> + * inspect the state required for watermark updates. >> + * >> + * Below we'll grab the required crtc state. >> + */ >> + ret = drm_modeset_lock(&dev->mode_config.connection_mutex, >> + state->acquire_ctx); > Do we need to get lock here? > We will anyway go ahead & get the connection_mutex lock during skl_compute_ddb based on distruct_bios_wm flag. Oh right, missed that one, yeah ignore it. :) I'll send the original patch again, thanks for catching. ~Maarten