Re: [PATCH V2 1/3] drm/vkms/crc: Implement verify_crc_source callback

[Haneen Mohammed <hamohammed.sa@gmail.com>]

On Tue, Aug 14, 2018 at 08:31:03AM +0530, Mahesh Kumar wrote:
> This patch implements "verify_crc_source" callback function for
> Virtual KMS drm driver.
> 
> Changes Since V1:
> - update values_cnt in verify_crc_source
> 
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/vkms/vkms_crc.c  | 38 ++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/vkms/vkms_crtc.c |  1 +
>  drivers/gpu/drm/vkms/vkms_drv.h  |  2 ++
>  3 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 37d717f38e3c..b2a484b4e2ad 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
>  	drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
>  }
>  
> +static int vkms_crc_parse_source(const char *src_name, bool *enabled)
> +{
> +	int ret = 0;
> +
> +	if (!src_name) {
> +		*enabled = false;
> +	} else if (strcmp(src_name, "auto") == 0) {
> +		*enabled = true;
> +	} else {
> +		*enabled = false;
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
> +			   size_t *values_cnt)
> +{
> +	bool enabled;
> +
> +	if (vkms_crc_parse_source(src_name, &enabled) < 0) {
> +		DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
> +		return -EINVAL;
> +	}
> +
> +	*values_cnt = 1;
> +
> +	return 0;
> +}
> +
>  int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  			size_t *values_cnt)
>  {
> @@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  	unsigned long flags;
>  	int ret = 0;
>  
> -	if (src_name && strcmp(src_name, "auto") == 0)
> -		enabled = true;
> -	else if (src_name)
> -		ret = -EINVAL;
> +	ret = vkms_crc_parse_source(src_name, &enabled);
> +	if (ret)
> +		return ret;

I think the return value after vkms_crc_parse_source should be called
once instead at the end of vkms_set_crc_source otherwise crc_enabled
won't be updated?

- Haneen

>  
>  	*values_cnt = 1;
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
> index bfe6e0312cc4..9d0b1a325a78 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
>  	.enable_vblank		= vkms_enable_vblank,
>  	.disable_vblank		= vkms_disable_vblank,
>  	.set_crc_source		= vkms_set_crc_source,
> +	.verify_crc_source	= vkms_verify_crc_source,
>  };
>  
>  static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index f156c930366a..090c5e4f5544 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
>  /* CRC Support */
>  int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
>  			size_t *values_cnt);
> +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
> +			   size_t *values_cnt);
>  void vkms_crc_work_handle(struct work_struct *work);
>  
>  #endif /* _VKMS_DRV_H_ */
> -- 
> 2.16.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx