Re: [PATCH 3/5] drm/i915: Fix watermarks for VLV/CHV

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 10-03-16 om 11:33 schreef Ville Syrjälä:
> On Thu, Mar 10, 2016 at 10:09:40AM +0100, Maarten Lankhorst wrote:
>> Op 09-03-16 om 18:07 schreef ville.syrjala@linux.intel.com:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> commit 92826fcdfc14 ("drm/i915: Calculate watermark related members in the crtc_state, v4.")
>>> broke thigns by removing the pre vs. post wm update distinction. We also
>>> lost the pre plane wm update entirely for VLV/CHV from the crtc enable
>>> path.
>>>
>>> This caused underruns on modeset and plane enable/disable on CHV,
>>> and often those can lead to a dead pipe.
>>>
>>> So let's bring back the pre vs. post thing, and let's toss in an
>>> explicit wm update to valleyview_crtc_enable() to avoid having to
>>> put it into the common code.
>>>
>>> This is more or less a partial revert of the offending commit.
>>>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: drm-intel-fixes@lists.freedesktop.org
>>> Fixes: 92826fcdfc14 ("drm/i915: Calculate watermark related members in the crtc_state, v4.")
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_atomic.c  |  3 ++-
>>>  drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++----------
>>>  drivers/gpu/drm/i915/intel_drv.h     |  2 +-
>>>  3 files changed, 22 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>>> index 6a661e796328..79448f1c8b8d 100644
>>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>>> @@ -96,7 +96,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>>>  	crtc_state->update_pipe = false;
>>>  	crtc_state->disable_lp_wm = false;
>>>  	crtc_state->disable_cxsr = false;
>>> -	crtc_state->wm_changed = false;
>>> +	crtc_state->update_wm_pre = false;
>>> +	crtc_state->update_wm_post = false;
>>>  	crtc_state->fb_changed = false;
>>>  	crtc_state->wm.need_postvbl_update = false;
>> There's already a wm.need_postvbl_update.
>> could we use it for non-atomic platforms, and add a wm.need_intermediate_update ?
>> This could probably be used for ILK too.
> Dunno. All I know is the regression is now on it's way 4.5 and needs to
> be dealt with ASAP. So no major overhaul at this point.
>
Ok, since I lack the hardware to test for now this will have to do.. I'd like this to be revisited in the future.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx