From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Damien Lespiau <damien.lespiau@intel.com>,
Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/i915: Don't forget to mark crtc as inactive after disable
Date: Mon, 13 Jul 2015 11:10:51 +0200 [thread overview]
Message-ID: <55A3809B.80102@linux.intel.com> (raw)
In-Reply-To: <20150710112251.GC4901@strange.ger.corp.intel.com>
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@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@linux.intel.com>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>> Tested-by(IVB): Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> v2: Don't touch disable_shared_dpll()
>>
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@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@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-07-13 9:11 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-08 13:31 [PATCH v2] drm/i915: Don't forget to mark crtc as inactive after disable Patrik Jakobsson
2015-07-08 17:24 ` shuang.he
2015-07-10 11:22 ` Damien Lespiau
2015-07-10 11:56 ` Patrik Jakobsson
2015-07-13 9:10 ` Maarten Lankhorst [this message]
2015-07-13 9:49 ` Daniel Vetter
2015-07-13 10:42 ` Damien Lespiau
2015-07-13 14:53 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55A3809B.80102@linux.intel.com \
--to=maarten.lankhorst@linux.intel.com \
--cc=damien.lespiau@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=patrik.jakobsson@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox