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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>
>> ---
>>  .../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()?

>> +
>>  	return &intel_state->uapi;
>>  }
>>  
>> @@ -112,6 +115,8 @@ intel_plane_destroy_state(struct drm_plane *plane,
>>  	WARN_ON(plane_state->vma);
>>  
>>  	__drm_atomic_helper_plane_destroy_state(&plane_state->uapi);
>> +	if (plane_state->hw.fb)
>> +		drm_framebuffer_put(plane_state->hw.fb);
>>  	kfree(plane_state);
>>  }
>>  
>> @@ -138,15 +143,34 @@ unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
>>  	return cpp * crtc_state->pixel_rate;
>>  }
>>  
>> +void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
>> +				       const struct intel_plane_state *from_plane_state)
>> +{
>> +	plane_state->hw.crtc = from_plane_state->uapi.crtc;
>> +	plane_state->hw.fb = from_plane_state->uapi.fb;
>> +	if (plane_state->hw.fb)
>> +		drm_framebuffer_get(plane_state->hw.fb);
>> +
>> +	plane_state->hw.alpha = from_plane_state->uapi.alpha;
>> +	plane_state->hw.pixel_blend_mode =
>> +		from_plane_state->uapi.pixel_blend_mode;
>> +	plane_state->hw.rotation = from_plane_state->uapi.rotation;
>> +	plane_state->hw.color_encoding = from_plane_state->uapi.color_encoding;
>> +	plane_state->hw.color_range = from_plane_state->uapi.color_range;
>> +}
>> +
>>  int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_state,
>>  					struct intel_crtc_state *new_crtc_state,
>>  					const struct intel_plane_state *old_plane_state,
>>  					struct intel_plane_state *new_plane_state)
>>  {
>>  	struct intel_plane *plane = to_intel_plane(new_plane_state->uapi.plane);
>> -	const struct drm_framebuffer *fb = new_plane_state->hw.fb;
>> +	const struct drm_framebuffer *fb;
>>  	int ret;
>>  
>> +	intel_plane_copy_uapi_to_hw_state(new_plane_state, new_plane_state);
>> +	fb = new_plane_state->hw.fb;
>> +
>>  	new_crtc_state->active_planes &= ~BIT(plane->id);
>>  	new_crtc_state->nv12_planes &= ~BIT(plane->id);
>>  	new_crtc_state->c8_planes &= ~BIT(plane->id);
>> @@ -373,6 +397,7 @@ intel_atomic_get_plane_state_after_check(struct intel_atomic_state *state,
>>  {
>>  	struct intel_plane_state *plane_state =
>>  		intel_atomic_get_new_plane_state(state, plane);
>> +	const struct intel_plane_state *old_plane_state;
>>  
>>  	if (plane_state)
>>  		return plane_state;
>> @@ -381,6 +406,16 @@ intel_atomic_get_plane_state_after_check(struct intel_atomic_state *state,
>>  	if (IS_ERR(plane_state))
>>  		return plane_state;
>>  
>> +	old_plane_state = intel_atomic_get_old_plane_state(state, plane);
>> +
>> +	/*
>> +	 * copy HW parameters since they're zero'd on duplication, to prevent
>> +	 * accidentally using stale state.
>> +	 */
>> +	plane_state->hw = old_plane_state->hw;
>> +	if (plane_state->hw.fb)
>> +		drm_framebuffer_get(plane_state->hw.fb);
>> +
>>  	new_crtc_state->update_planes |= BIT(plane->id);
>>  	return plane_state;
>>  }
>> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.h b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
>> index cb7ef4f9eafd..fd4b96013454 100644
>> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.h
>> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.h
>> @@ -20,6 +20,8 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>>  
>>  unsigned int intel_plane_data_rate(const struct intel_crtc_state *crtc_state,
>>  				   const struct intel_plane_state *plane_state);
>> +void intel_plane_copy_uapi_to_hw_state(struct intel_plane_state *plane_state,
>> +				       const struct intel_plane_state *from_plane_state);
>>  void intel_update_plane(struct intel_plane *plane,
>>  			const struct intel_crtc_state *crtc_state,
>>  			const struct intel_plane_state *plane_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 0ee542840c70..ffde4b8b8be9 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -3267,6 +3267,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>>  
>>  	plane_state->fb = fb;
>>  	plane_state->crtc = &intel_crtc->base;
>> +	intel_plane_copy_uapi_to_hw_state(intel_state, intel_state);
>>  
>>  	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
>>  		  &to_intel_frontbuffer(fb)->bits);
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index c6c49aa8c172..67644818efd2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -515,10 +515,24 @@ struct intel_atomic_state {
>>  };
>>  
>>  struct intel_plane_state {
>> -	union {
>> -		struct drm_plane_state uapi;
>> -		struct drm_plane_state hw;
>> -	};
>> +	struct drm_plane_state uapi;
>> +
>> +	/*
>> +	 * actual hardware state, the state we program to the hardware.
>> +	 * The following members are used to verify the hardware state:
>> +	 * During initial hw readout, they need to be copied from uapi.
>> +	 */
>> +	struct {
>> +		struct drm_crtc *crtc;
>> +		struct drm_framebuffer *fb;
>> +
>> +		u16 alpha;
>> +		uint16_t pixel_blend_mode;
>> +		unsigned int rotation;
>> +		enum drm_color_encoding color_encoding;
>> +		enum drm_color_range color_range;
>> +	} hw;
>> +
>>  	struct i915_ggtt_view view;
>>  	struct i915_vma *vma;
>>  	unsigned long flags;
>> -- 
>> 2.23.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux