[Intel-gfx] [PATCH v2 2/9] drm/i915: Kill off intel_crtc->atomic.wait_vblank, v3.
maarten.lankhorst at linux.intel.com
Tue Feb 9 11:07:01 UTC 2016
Op 09-02-16 om 11:42 schreef Ville Syrjälä:
> On Tue, Feb 09, 2016 at 11:27:44AM +0100, Maarten Lankhorst wrote:
>> Op 13-01-16 om 13:58 schreef Ville Syrjälä:
>>> On Mon, Jan 11, 2016 at 01:27:42PM +0100, Maarten Lankhorst wrote:
>>>> Currently we perform our own wait in post_plane_update,
>>>> but the atomic core performs another one in wait_for_vblanks.
>>>> This means that 2 vblanks are done when a fb is changed,
>>>> which is a bit overkill.
>>>> Merge them by creating a helper function that takes a crtc mask
>>>> for the planes to wait on.
>>>> The broadwell vblank workaround may look gone entirely but this is
>>>> not the case. pipe_config->wm_changed is set to true
>>>> when any plane is turned on, which forces a vblank wait.
>>> Still NAK, because you just removed the comment entirely.
>> I may have removed the comment but there will always be an unconditional wait because of the wm changes.
>> In some future commit I will rework do_intel_finish_page_flip to deal with atomic, and in that function the comment
>> will be useful again.
> The comment is the spec here, so it should be kept. Actually what I
> really want is to stop using the flip done interrupt entirely since
> it's arguably broken, at which point the comment should problably be
> moved to somewhere else (interrupt setup code, flip completion code,
> etc.). But removing the comment would be bad since then people might
> not understand why we do it the way we do.
I think the flip done interrupt will still be needed for cs flips, but Chris was against removing it iirc. I'll move the comment to page_flip_finished for now.
>>>> Changes since v1:
>>>> - Removing the double vblank wait on broadwell moved to its own commit.
>>>> Changes since v2:
>>>> - Move out POWER_DOMAIN_MODESET handling to its own commit.
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>>> drivers/gpu/drm/i915/intel_atomic.c | 1 +
>>>> drivers/gpu/drm/i915/intel_display.c | 80 ++++++++++++++++++++++++++----------
>>>> drivers/gpu/drm/i915/intel_drv.h | 2 +-
>>>> 3 files changed, 61 insertions(+), 22 deletions(-)
>>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>>>> index 9682d94af710..ba9a57f33c43 100644
>>>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>>>> @@ -98,6 +98,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>>>> crtc_state->disable_cxsr = false;
>>>> crtc_state->wm_changed = false;
>>>> crtc_state->wm.need_postvbl_update = false;
>>>> + crtc_state->fb_changed = false;
>>>> return &crtc_state->base;
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index 2aa1c5367625..eac73f005a70 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -4796,9 +4796,6 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>>>> struct drm_device *dev = crtc->base.dev;
>>>> - if (atomic->wait_vblank)
>>>> - intel_wait_for_vblank(dev, crtc->pipe);
>>>> intel_frontbuffer_flip(dev, atomic->fb_bits);
>>>> crtc->wm.cxsr_allowed = true;
>>>> @@ -11872,6 +11869,9 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>>> if (!was_visible && !visible)
>>>> return 0;
>>>> + if (fb != old_plane_state->base.fb)
>>>> + pipe_config->fb_changed = true;
>>> Isn't that going to slow down cursor updates once again?
>> Very likely.. Shall I add a if (!state->legacy_cursor_update) to intel_atomic_wait_for_vblanks ?
> Not sure. Still wishing we could have just had the proper fps>=vrefresh
> support fron the start.
Working on it!
More information about the Intel-gfx