Re: [PATCH 12/24] drm/i915: Split plane hw and uapi state

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 12/24] drm/i915: Split plane hw and uapi state
Date: Wed, 9 Oct 2019 14:31:58 +0200	[thread overview]
Message-ID: <4c2b6570-4757-df80-9d5f-904d2e649593@linux.intel.com> (raw)
In-Reply-To: <20191009122305.GY1208@intel.com>

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@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. :)

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx