Re: [PATCH 1/4] drm/atomic: Save flip flags in drm_plane_state

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jan 16, 2017 at 10:44:55AM -0500, Andrey Grodzovsky wrote:
> Allows using atomic flip helpers for drivers
> using ASYNC flip.
> Remove ASYNC_FLIP restriction in helpers and
> caches the page flip flags in drm_plane_state
> to be used in the low level drivers.
> 
> Signed-off-by: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 10 +++-------
>  include/drm/drm_plane.h             |  8 ++++++++
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index a4e5477..f83dc43 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2737,7 +2737,8 @@ static int page_flip_common(
>  				struct drm_atomic_state *state,
>  				struct drm_crtc *crtc,
>  				struct drm_framebuffer *fb,
> -				struct drm_pending_vblank_event *event)
> +				struct drm_pending_vblank_event *event,
> +				uint32_t flags)
>  {
>  	struct drm_plane *plane = crtc->primary;
>  	struct drm_plane_state *plane_state;
> @@ -2754,6 +2755,7 @@ static int page_flip_common(
>  	if (IS_ERR(plane_state))
>  		return PTR_ERR(plane_state);
>  
> +	plane_state->pflip_flags = flags;

"pflip" looks off. Better just spell it out.

These flags need to be reset in duplicate_state otherwise they leak into
subsequent operations. This is why I don't really like the concept of
"state" containing flags and stuff that are not really part of the
state but rather part of the operation of moving from the old state to
the new state. But maybe we don't have a better place for this sort of
stuff? I have suggested at some point that we might rename drm_atomic_state
to drm_atomic_transaction or something. Stuffing the flags (or just a bool
perhaps?) in there might be less confusing.

>  
>  	ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
>  	if (ret != 0)
> @@ -2800,9 +2802,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>  	struct drm_atomic_state *state;
>  	int ret = 0;
>  
> -	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> -		return -EINVAL;
> -
>  	state = drm_atomic_state_alloc(plane->dev);
>  	if (!state)
>  		return -ENOMEM;
> @@ -2865,9 +2864,6 @@ int drm_atomic_helper_page_flip_target(
>  	struct drm_crtc_state *crtc_state;
>  	int ret = 0;
>  
> -	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> -		return -EINVAL;
> -
>  	state = drm_atomic_state_alloc(plane->dev);
>  	if (!state)
>  		return -ENOMEM;
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index db3bbde..86d8ffc 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -122,6 +122,14 @@ struct drm_plane_state {
>  	 */
>  	bool visible;
>  
> +
> +	/**
> +	 * @pflip_flags:
> +	 *
> +	 * Flip related config options
> +	 */
> +	u32 pflip_flags;
> +
>  	struct drm_atomic_state *state;
>  };
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel