[Intel-gfx] [PATCH 12/24] drm/i915: Split plane hw and uapi state

[Ville Syrjälä]

On Wed, Oct 09, 2019 at 02:31:58PM +0200, Maarten Lankhorst wrote:
> Op 09-10-2019 om 14:23 schreef Ville Syrjälä:
> > On Wed, Oct 09, 2019 at 02:13:55PM +0200, Maarten Lankhorst wrote:
> >> Op 08-10-2019 om 19:42 schreef Ville Syrjälä:
> >>> On Fri, Oct 04, 2019 at 01:35:02PM +0200, Maarten Lankhorst wrote:
> >>>> Splitting plane state is easier than splitting crtc_state,
> >>>> before plane check we copy the drm properties to hw so we can
> >>>> do the same in bigjoiner later on.
> >>>>
> >>>> We copy the state after we did all the modeset handling, but fortunately
> >>>> i915 seems to be split correctly and nothing during modeset looks
> >>>> at plane_state.
> >>>>
> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> >>>> ---
> >>>>  .../gpu/drm/i915/display/intel_atomic_plane.c | 37 ++++++++++++++++++-
> >>>>  .../gpu/drm/i915/display/intel_atomic_plane.h |  2 +
> >>>>  drivers/gpu/drm/i915/display/intel_display.c  |  1 +
> >>>>  .../drm/i915/display/intel_display_types.h    | 22 +++++++++--
> >>>>  4 files changed, 57 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >>>> index 01937896d69c..cc154cfa3381 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> >>>> @@ -93,6 +93,9 @@ intel_plane_duplicate_state(struct drm_plane *plane)
> >>>>  	intel_state->vma = NULL;
> >>>>  	intel_state->flags = 0;
> >>>>  
> >>>> +	/* will be set in intel_plane_atomic_check_with_state() */
> >>>> +	memset(&intel_state->hw, 0, sizeof(intel_state->hw));
> >>> That seems wrong for the case where we add the plane after the
> >>> plane check has already been done.
> >>>
> >>> I think we should maintain the current state unless the plane check
> >>> will overwrite it.
> >> Have you seen
> >>
> >> intel_atomic_get_plane_state_after_check()?
> > Yes, but I don't like it.
> 
> This is however required because of the split. We should not look at the old plane_state contents before atomic_check and to prevent that I think it's better to zero out plane_state->hw,
> 
> in which case bugs are at least consistent. :)

I don't like leaking this special case all over. IMO it's not really any
different to the current rule that you can't look at any of the derived
state before plane check(). We're just extending the concept of the
derived state a bit.

-- 
Ville Syrjälä
Intel