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

[Maarten Lankhorst]

Op 09-10-2019 om 14:41 schreef 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.
>
I'm ok with that..