Re: [PATCH 3/3] drm/atomic: Rename crtc_state->pageflip_flags to async_flip

["Kazlauskas, Nicholas" <Nicholas.Kazlauskas@amd.com>]

On 2019-09-03 3:06 p.m., Daniel Vetter wrote:
> It's the only flag anyone actually cares about. Plus if we're unlucky,
> the atomic ioctl might need a different flag for async flips. So
> better to abstract this away from the uapi a bit.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Michel Dänzer <michel@daenzer.net>
> Cc: Alex Deucher <alexdeucher@gmail.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Leo Li <sunpeng.li@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: David Francis <David.Francis@amd.com>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Series is:

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

I would like to see a new flag eventually show up for atomic as well, 
but the existing one is effectively broken at this point and I would 
hope that no userspace is setting it expecting that it actually does 
something.

At this point we don't really gain anything from enabling atomic in DDX 
I think, most drivers already make use of DRM helpers to map these 
legacy IOCTLs to atomic anyway.

Nicholas Kazlauskas

>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++---
>   drivers/gpu/drm/drm_atomic_helper.c               | 2 +-
>   drivers/gpu/drm/drm_atomic_state_helper.c         | 2 +-
>   drivers/gpu/drm/nouveau/dispnv50/wndw.c           | 4 ++--
>   include/drm/drm_crtc.h                            | 8 ++++----
>   5 files changed, 10 insertions(+), 11 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 0a71ed1e7762..2f0ef0820f00 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5756,8 +5756,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   		 * change FB pitch, DCC state, rotation or mirroing.
>   		 */
>   		bundle->flip_addrs[planes_count].flip_immediate =
> -			(crtc->state->pageflip_flags &
> -			 DRM_MODE_PAGE_FLIP_ASYNC) != 0 &&
> +			crtc->state->async_flip &&
>   			acrtc_state->update_type == UPDATE_TYPE_FAST;
>   
>   		timestamp_ns = ktime_get_ns();
> @@ -6334,7 +6333,7 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
>   	amdgpu_dm_enable_crtc_interrupts(dev, state, true);
>   
>   	for_each_new_crtc_in_state(state, crtc, new_crtc_state, j)
> -		if (new_crtc_state->pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC)
> +		if (new_crtc_state->async_flip)
>   			wait_for_vblank = false;
>   
>   	/* update planes when needed per crtc*/
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index e9c6112e7f73..1e5293eb66e3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3263,7 +3263,7 @@ static int page_flip_common(struct drm_atomic_state *state,
>   		return PTR_ERR(crtc_state);
>   
>   	crtc_state->event = event;
> -	crtc_state->pageflip_flags = flags;
> +	crtc_state->async_flip = flags & DRM_MODE_PAGE_FLIP_ASYNC;
>   
>   	plane_state = drm_atomic_get_plane_state(state, plane);
>   	if (IS_ERR(plane_state))
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 46dc264a248b..d0a937fb0c56 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -128,7 +128,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>   	state->zpos_changed = false;
>   	state->commit = NULL;
>   	state->event = NULL;
> -	state->pageflip_flags = 0;
> +	state->async_flip = false;
>   
>   	/* Self refresh should be canceled when a new update is available */
>   	state->active = drm_atomic_crtc_effectively_active(state);
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> index 2db029371c91..5193b6257061 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> @@ -267,7 +267,7 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw, bool modeset,
>   			asyw->image.pitch[0] = fb->base.pitches[0];
>   		}
>   
> -		if (!(asyh->state.pageflip_flags & DRM_MODE_PAGE_FLIP_ASYNC))
> +		if (!asyh->state.async_flip)
>   			asyw->image.interval = 1;
>   		else
>   			asyw->image.interval = 0;
> @@ -383,7 +383,7 @@ nv50_wndw_atomic_check_lut(struct nv50_wndw *wndw,
>   	}
>   
>   	/* Can't do an immediate flip while changing the LUT. */
> -	asyh->state.pageflip_flags &= ~DRM_MODE_PAGE_FLIP_ASYNC;
> +	asyh->state.async_flip = false;
>   }
>   
>   static int
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 7e2963cad543..900ae8d452b8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -284,12 +284,12 @@ struct drm_crtc_state {
>   	u32 target_vblank;
>   
>   	/**
> -	 * @pageflip_flags:
> +	 * @async_flip:
>   	 *
> -	 * DRM_MODE_PAGE_FLIP_* flags, as passed to the page flip ioctl.
> -	 * Zero in any other case.
> +	 * This is set when DRM_MODE_PAGE_FLIP_ASYNC is set in the legacy
> +	 * PAGE_FLIP IOCTL. It's not wired up for the atomic IOCTL itself yet.
>   	 */
> -	u32 pageflip_flags;
> +	bool async_flip;
>   
>   	/**
>   	 * @vrr_enabled:
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel