[Intel-gfx] [PATCH v2] drm/i915: Don't forget to mark crtc as inactive after disable

[Maarten Lankhorst]

Op 10-07-15 om 13:22 schreef Damien Lespiau:
> Hi Patrik,
>
> Please do Cc the patch author and reviewer when finding a regression,
> they are superb candidates for the review, especially when they are busy
> rewriting the display code.
>
> On Wed, Jul 08, 2015 at 03:31:52PM +0200, Patrik Jakobsson wrote:
>> Watermark calculations depend on the intel_crtc->active flag to be set
>> properly. Suspend/resume is broken on SKL and we also get DDB mismatches
>> without this patch.
>>
>> The regression was introduced in:
>>
>> commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
>> Author: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> Date:   Mon Jun 15 12:33:53 2015 +0200
>>
>>     drm/i915: Update less state during modeset.
>>
>>     No need to repeatedly call update_watermarks, or update_fbc.
>>     Down to a single call to update_watermarks in .crtc_enable
>>
>>     Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>     Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>>     Tested-by(IVB): Matt Roper <matthew.d.roper at intel.com>
>>     Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>
>> v2: Don't touch disable_shared_dpll()
>>
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> We do need to update the watermarks in disable() as well, to repartition
> the DDB and update the watermarks based on the new allocation.
>
> Maarten, Matt, I've no clue where you want the crtc state update, where
> the atomic WM work is at, could you comment?
>
I'd rather have we had a better way of updating watermarks, but keeping it in
the .crtc_disable hook looks good to me right now. Some proper atomic
solution will eventually have to be done. :)

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