Re: [PATCH 06/10] drm/i915: Update legacy primary state outside the commit hook.

From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 06/10] drm/i915: Update legacy primary state outside the commit hook.
Date: Mon, 14 Sep 2015 16:26:53 +0200	[thread overview]
Message-ID: <20150914142653.GM3383@phenom.ffwll.local> (raw)
In-Reply-To: <55F6C8A4.4010004@linux.intel.com>

On Mon, Sep 14, 2015 at 03:16:20PM +0200, Maarten Lankhorst wrote:
> Op 14-09-15 om 15:13 schreef Daniel Vetter:
> > On Mon, Sep 14, 2015 at 11:52:26AM +0200, Maarten Lankhorst wrote:
> >> Op 10-09-15 om 17:41 schreef Daniel Vetter:
> >>> On Thu, Sep 10, 2015 at 04:08:01PM +0200, Maarten Lankhorst wrote:
> >>>> This should allow not running plane commit when the crtc is off.
> >>>> While the atomic helpers update those, crtc->x/y is only updated
> >>>> during modesets, and primary plane is updated after this function
> >>>> returns.
> >>>>
> >>>> Unfortunately non-atomic watermarks and fbc still depend on this
> >>>> state inside i915, so it has to be kept in sync.
> >>>>
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>>> ---
> >>>>  drivers/gpu/drm/i915/intel_display.c | 15 ++++++++-------
> >>>>  1 file changed, 8 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index 87c5eba08454..b809ee2a8678 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -12198,6 +12198,14 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state)
> >>>>  			crtc->hwmode = crtc->state->adjusted_mode;
> >>>>  		else
> >>>>  			crtc->hwmode.crtc_clock = 0;
> >>>> +
> >>>> +		if (drm_atomic_get_existing_plane_state(state, crtc->primary)) {
> >>>> +			struct drm_plane_state *plane_state = crtc->primary->state;
> >>>> +
> >>>> +			crtc->primary->fb = plane_state->fb;
> >>>> +			crtc->x = plane_state->src_x >> 16;
> >>>> +			crtc->y = plane_state->src_y >> 16;
> >>>> +		}
> >>> drm_atomic_helper_update_legacy_modeset_state should do that for us, not?
> >>> Atm we have an if (any_ms) check for it, but that seems to just be a bug
> >>> really.
> >> That function doesn't update primary->fb though. This is fixed up by the caller of drm_atomic_commit
> >> after the call succeeds. We need it sooner because update_fbc unfortunately still looks at the legacy state.
> >>
> >> There are a few other users of the legacy state left in i915, but they should be more easy to convert than fbc.
> > Hm, I tried looking really hard but didn't find anything that cared about
> > plane->fb. Can you please point me at them?
> >
> Look for primary->fb. :-) Also crtc->y is used in __intel_fbc_update.

Well I did hunt for that but only found usage in crtc_disable functions
(where we need the old value anyway) and flip code (which updates it
itself). I did indeed somehow miss the fbc code. Oh well. Comment might be
good for that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx