[PATCH] drm/amd/display: remove need of modeset flag for overlay planes

[Harry Wentland]

On 2018-04-27 06:27 AM, Shirish S wrote:
> This patch is in continuation to the
> "843e3c7 drm/amd/display: defer modeset check in dm_update_planes_state"
> where we started to eliminate the dependency on
> DRM_MODE_ATOMIC_ALLOW_MODESET to be set by the user space,
> which as such is not mandatory.
> 
> After deferring, this patch eliminates the dependency on the flag
> for overlay planes.
> 

Apologies for the late response. I had to think about this patch for a long time since I'm not quite comfortable with it.

> This has to be done in stages as its a pretty complex and requires thorough
> testing before we free primary planes as well from dependency on modeset
> flag.
> 
> Signed-off-by: Shirish S <shirish.s at amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 1a63c04..87b661d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4174,7 +4174,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>  		}
>  		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>  
> -		if (!pflip_needed) {
> +		if (!pflip_needed || plane->type == DRM_PLANE_TYPE_OVERLAY) {

Does this mean that whenever we have an overlay plane we won't do amdgpu_dm_do_flip but commit_planes_to_stream instead? Is this really the behavior we want?

commit_planes_to_stream was intended to program a new surface on a modeset whereas amdgpu_dm_do_flip was intended for pageflips. If we use commit_planes_to_stream we end up losing things like the immediate_flip flag, as well as the wait for the right moment to program the flip that amdgpu_dm_do_flip does. Even more importantly we won't wait for fences (reservation_object_wait_timeout_rcu).

Harry

>  			WARN_ON(!dm_new_plane_state->dc_state);
>  
>  			plane_states_constructed[planes_count] = dm_new_plane_state->dc_state;
> @@ -4884,7 +4884,8 @@ static int dm_update_planes_state(struct dc *dc,
>  
>  		/* Remove any changed/removed planes */
>  		if (!enable) {
> -			if (pflip_needed)
> +			if (pflip_needed &&
> +			    plane && plane->type != DRM_PLANE_TYPE_OVERLAY)
>  				continue;
>  
>  			if (!old_plane_crtc)
> @@ -4931,7 +4932,8 @@ static int dm_update_planes_state(struct dc *dc,
>  			if (!dm_new_crtc_state->stream)
>  				continue;
>  
> -			if (pflip_needed)
> +			if (pflip_needed &&
> +			    plane && plane->type != DRM_PLANE_TYPE_OVERLAY)
>  				continue;
>  
>  			WARN_ON(dm_new_plane_state->dc_state);
>