[PATCH 09/10] drm/crc: Cleanup crtc_crc_open function
Kumar, Mahesh
mahesh1.kumar at intel.com
Tue Jun 26 06:33:08 UTC 2018
Cc: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
On 6/26/2018 11:52 AM, Mahesh Kumar wrote:
> This patch make changes to allocate crc-entries buffer before
> enabling CRC generation.
> It moves all the failure check early in the function before setting
> the source or memory allocation.
> Now set_crc_source takes only two variable input, values_cnt we
> already gets as part of verify_crc_source.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> Cc: dri-devel at lists.freedesktop.org
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 3 +-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 4 +-
> drivers/gpu/drm/drm_debugfs_crc.c | 52 +++++++++-------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_pipe_crc.c | 4 +-
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 5 +--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +--
> include/drm/drm_crtc.h | 3 +-
> 8 files changed, 30 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 79b0a16652b9..a7a5551fee1b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -261,8 +261,7 @@ amdgpu_dm_remove_sink_from_freesync_module(struct drm_connector *connector);
>
> /* amdgpu_dm_crc.c */
> #ifdef CONFIG_DEBUG_FS
> -int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> - size_t *values_cnt);
> +int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name);
> int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc,
> const char *src_name,
> size_t *values_cnt);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
> index dfcca594d52a..e7ad528f5853 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
> @@ -62,8 +62,7 @@ amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
> return 0;
> }
>
> -int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> - size_t *values_cnt)
> +int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name)
> {
> struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state);
> struct dc_stream_state *stream_state = crtc_state->stream;
> @@ -99,7 +98,6 @@ int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> return -EINVAL;
> }
>
> - *values_cnt = 3;
> /* Reset crc_skipped on dm state */
> crtc_state->crc_skip_count = 0;
> return 0;
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index 2751d124387d..834bc7ee5550 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -109,11 +109,9 @@ static ssize_t crc_control_write(struct file *file, const char __user *ubuf,
> if (source[len] == '\n')
> source[len] = '\0';
>
> - if (crtc->funcs->verify_crc_source) {
> - ret = crtc->funcs->verify_crc_source(crtc, source, &values_cnt);
> - if (ret)
> - return ret;
> - }
> + ret = crtc->funcs->verify_crc_source(crtc, source, &values_cnt);
> + if (ret)
> + return ret;
>
> spin_lock_irq(&crc->lock);
>
> @@ -178,12 +176,15 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
> return ret;
> }
>
> - if (crtc->funcs->verify_crc_source) {
> - ret = crtc->funcs->verify_crc_source(crtc, crc->source,
> - &values_cnt);
> - if (ret)
> - return ret;
> - }
> + ret = crtc->funcs->verify_crc_source(crtc, crc->source, &values_cnt);
> + if (ret)
> + return ret;
> +
> + if (WARN_ON(values_cnt > DRM_MAX_CRC_NR))
> + return -EINVAL;
> +
> + if (WARN_ON(values_cnt == 0))
> + return -EINVAL;
>
> spin_lock_irq(&crc->lock);
> if (!crc->opened)
> @@ -195,30 +196,22 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
> if (ret)
> return ret;
>
> - ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);
> - if (ret)
> - goto err;
> -
> - if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) {
> - ret = -EINVAL;
> - goto err_disable;
> - }
> -
> - if (WARN_ON(values_cnt == 0)) {
> - ret = -EINVAL;
> - goto err_disable;
> - }
> -
> entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
> if (!entries) {
> ret = -ENOMEM;
> - goto err_disable;
> + goto err;
> }
>
> spin_lock_irq(&crc->lock);
> crc->entries = entries;
> crc->values_cnt = values_cnt;
> + spin_unlock_irq(&crc->lock);
>
> + ret = crtc->funcs->set_crc_source(crtc, crc->source);
> + if (ret)
> + goto err;
> +
> + spin_lock_irq(&crc->lock);
> /*
> * Only return once we got a first frame, so userspace doesn't have to
> * guess when this particular piece of HW will be ready to start
> @@ -235,7 +228,7 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
> return 0;
>
> err_disable:
> - crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);
> + crtc->funcs->set_crc_source(crtc, NULL);
> err:
> spin_lock_irq(&crc->lock);
> crtc_crc_cleanup(crc);
> @@ -247,9 +240,8 @@ static int crtc_crc_release(struct inode *inode, struct file *filep)
> {
> struct drm_crtc *crtc = filep->f_inode->i_private;
> struct drm_crtc_crc *crc = &crtc->crc;
> - size_t values_cnt;
>
> - crtc->funcs->set_crc_source(crtc, NULL, &values_cnt);
> + crtc->funcs->set_crc_source(crtc, NULL);
>
> spin_lock_irq(&crc->lock);
> crtc_crc_cleanup(crc);
> @@ -363,7 +355,7 @@ int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
> {
> struct dentry *crc_ent, *ent;
>
> - if (!crtc->funcs->set_crc_source)
> + if (!crtc->funcs->set_crc_source || !crtc->funcs->verify_crc_source)
> return 0;
>
> crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 17735cafdd72..ba3f1c5e6a1e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2152,8 +2152,7 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
> /* intel_pipe_crc.c */
> int intel_pipe_crc_create(struct drm_minor *minor);
> #ifdef CONFIG_DEBUG_FS
> -int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> - size_t *values_cnt);
> +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name);
> int intel_crtc_verify_crc_source(struct drm_crtc *crtc,
> const char *source_name, size_t *values_cnt);
> void intel_crtc_get_crc_sources(struct seq_file *m, struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index d6807155a237..50bdb56afc79 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -1033,8 +1033,7 @@ int intel_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
> return -EINVAL;
> }
>
> -int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> - size_t *values_cnt)
> +int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[crtc->index];
> @@ -1073,7 +1072,6 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> }
>
> pipe_crc->skipped = 0;
> - *values_cnt = 5;
>
> out:
> intel_display_power_put(dev_priv, power_domain);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 24eeaa7e14d7..829c644addba 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -796,8 +796,7 @@ static int rcar_du_crtc_verify_crc_source(struct drm_crtc *crtc,
> }
>
> static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
> - const char *source_name,
> - size_t *values_cnt)
> + const char *source_name)
> {
> struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> struct drm_modeset_acquire_ctx ctx;
> @@ -837,8 +836,6 @@ static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
> return -EINVAL;
> }
>
> - *values_cnt = 1;
> -
> /* Perform an atomic commit to set the CRC source. */
> drm_modeset_acquire_init(&ctx, 0);
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index ea4884ac4cb0..ed7f8a2c7208 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1117,7 +1117,7 @@ static struct drm_connector *vop_get_edp_connector(struct vop *vop)
> }
>
> static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
> - const char *source_name, size_t *values_cnt)
> + const char *source_name)
> {
> struct vop *vop = to_vop(crtc);
> struct drm_connector *connector;
> @@ -1127,8 +1127,6 @@ static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
> if (!connector)
> return -EINVAL;
>
> - *values_cnt = 3;
> -
> if (source_name && strcmp(source_name, "auto") == 0)
> ret = analogix_dp_start_crc(connector);
> else if (!source_name)
> @@ -1153,7 +1151,7 @@ vop_crtc_verify_crc_source(struct drm_crtc *crtc, const char *source_name,
>
> #else
> static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
> - const char *source_name, size_t *values_cnt)
> + const char *source_name)
> {
> return -ENODEV;
> }
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b3824f92c190..257b3d738d94 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -659,8 +659,7 @@ struct drm_crtc_funcs {
> *
> * 0 on success or a negative error code on failure.
> */
> - int (*set_crc_source)(struct drm_crtc *crtc, const char *source,
> - size_t *values_cnt);
> + int (*set_crc_source)(struct drm_crtc *crtc, const char *source);
> /**
> * @verify_crc_source:
> *
More information about the dri-devel
mailing list