Re: [PATCH 08/11] drm/vmwgfx: Add and connect atomic state object check/commit

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Mar 27, 2017 at 03:01:01PM -0700, Sinclair Yeh wrote:
> This connects the main state object check and commit function.
> 
> Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 66 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 2250a34a..6b593aaf 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1581,8 +1581,74 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
>  	return &vfb->base;
>  }
>  
> +
> +
> +/**
> + * vmw_kms_atomic_check_modeset- validate state object for modeset changes
> + *
> + * @dev: DRM device
> + * @state: the driver state object
> + *
> + * This is a simple wrapper around drm_atomic_helper_check_modeset() for
> + * us to assign a value to mode->crtc_clock so that
> + * drm_calc_timestamping_constants() won't throw an error message
> + *
> + * RETURNS
> + * Zero for success or -errno
> + */
> +int
> +vmw_kms_atomic_check_modeset(struct drm_device *dev,
> +			     struct drm_atomic_state *state)
> +{
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_crtc *crtc;
> +	int i, ret;
> +
> +
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		if (crtc_state->mode.crtc_clock == 0) {
> +			/*
> +			 * Our virtual device does not have a dot clock,
> +			 * so use the logical clock value as the dot clock.
> +			 */
> +			crtc_state->mode.crtc_clock = crtc_state->mode.clock;

Can't you stuff this into your crtc->atomic_check function? At least I
don't see a depency here ... With that you could forgo this vmw version of
atomic_commit.

> +		}
> +	}
> +
> +	ret = drm_atomic_helper_check_modeset(dev, state);
> +	if (ret)
> +		return ret;
> +
> +	return drm_atomic_helper_check_planes(dev, state);
> +}
> +
> +
> +/**
> + * vmw_kms_atomic_commit - Perform an atomic state commit
> + *
> + * @dev: DRM device
> + * @state: the driver state object
> + * @nonblock: Whether nonblocking behaviour is requested
> + *
> + * This is a simple wrapper around drm_atomic_helper_commit() for
> + * us to clear the nonblocking value.
> + * Nonblocking commits should avoid waiting on GPU completion to return.
> + * That is always the case for us.

That's not all they do, they also avoid any other stalls. Atomic can do an
entire modeset in nonblocking mode.

Since the helpers provide you nonblocking commit for essentially for free
there's also no reason anymore to clear this, or at least I think it
should be a lot better than the one given here :-)
-Daniel

> + *
> + * RETURNS
> + * Zero for success or negative error code on failure.
> + */
> +int vmw_kms_atomic_commit(struct drm_device *dev,
> +			  struct drm_atomic_state *state,
> +			  bool nonblock)
> +{
> +	return drm_atomic_helper_commit(dev, state, false);
> +}
> +
>  static const struct drm_mode_config_funcs vmw_kms_funcs = {
>  	.fb_create = vmw_kms_fb_create,
> +	.atomic_check = vmw_kms_atomic_check_modeset,
> +	.atomic_commit = vmw_kms_atomic_commit,
>  };
>  
>  static int vmw_kms_generic_present(struct vmw_private *dev_priv,
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel