drm/crc: Cleanup crtc_crc_open function

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 inputs, values_cnt we
already gets as part of verify_crc_source.

Changes since V1:
 - refactor code to use single spin lock
Changes since V2:
 - rebase
Changes since V3:
 - rebase on top of VKMS driver

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Leo Li <sunpeng.li@amd.com> (V2)
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> (V3)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180821083858.26275-3-mahesh1.kumar@intel.com

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 e43ed064dc465dce5852b5f071434cbe27231cfa..54056d1800032e5c0999fe0c7c527ec4678f4ae1 100644 (file)
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -258,8 +258,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 dfcca594d52af3c337fe6c7d4ca36da0e6d5ce03..e7ad528f5853bfc410718e804fbfbce4177f0f38 100644 (file)
--- 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 d7e626331ecafb5ed85d7947fabe7e6042e909c8..3e0a2cfaa35c2edca50c03b6c01e86a73cd5db97 100644 (file)
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -127,11 +127,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);
 
@@ -197,40 +195,40 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
                        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;
+
+       entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL);
+       if (!entries)
+               return -ENOMEM;
+
        spin_lock_irq(&crc->lock);
-       if (!crc->opened)
+       if (!crc->opened) {
                crc->opened = true;
-       else
+               crc->entries = entries;
+               crc->values_cnt = values_cnt;
+       } else {
                ret = -EBUSY;
+       }
        spin_unlock_irq(&crc->lock);
 
-       if (ret)
+       if (ret) {
+               kfree(entries);
                return ret;
+       }
 
-       ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt);
+       ret = crtc->funcs->set_crc_source(crtc, crc->source);
        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;
-       }
-
        spin_lock_irq(&crc->lock);
-       crc->entries = entries;
-       crc->values_cnt = values_cnt;
-
        /*
         * 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
@@ -247,7 +245,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);
@@ -259,9 +257,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);
@@ -367,7 +364,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 27362214b77ba3cca0b5c09d9f5f9f1a89e79fb1..b41515bb9a15157cc6f2e4b8ea2bbe0e3bfb4e83 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2151,8 +2151,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);
 const char *const *intel_crtc_get_crc_sources(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 1dffc346f1bc1682fe4099e793ef65aeca06ee7e..27d560f7a817c5e305e95c7dca888469163f0cf6 100644 (file)
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -1028,8 +1028,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];
@@ -1068,7 +1067,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 57db868da4fe2f405d90dea0398722726f00a3a3..8a9e5e6f16b46ec8fea4a7aa6eeca39aa3c5b868 100644 (file)
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -887,8 +887,7 @@ const char *const *rcar_du_crtc_get_crc_sources(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;
@@ -903,7 +902,6 @@ static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
                return ret;
 
        index = ret;
-       *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 c9a5ea38e86bad1991fafb64cb50db4181f51bcc..38f8cae7ef514e1a0f119722c8e9040dc5c6df87 100644 (file)
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1111,7 +1111,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;
@@ -1121,8 +1121,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)
@@ -1146,7 +1144,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/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
index 68421d3d809ac7dcaaeefa94c8e486e8f82e9773..ed47d67cecd6943228da7d7a20b60d3be99f6052 100644 (file)
--- a/drivers/gpu/drm/vkms/vkms_crc.c
+++ b/drivers/gpu/drm/vkms/vkms_crc.c
@@ -101,8 +101,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
        return 0;
 }
 
-int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
-                       size_t *values_cnt)
+int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 {
        struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
        bool enabled = false;
@@ -111,8 +110,6 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
 
        ret = vkms_crc_parse_source(src_name, &enabled);
 
-       *values_cnt = 1;
-
        /* make sure nothing is scheduled on crtc workq */
        flush_workqueue(out->crc_workq);
 

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 090c5e4f55440e44b1dbfd94a129667a56a9c358..2017a2ccc43de88b34556f43baead30e1aa07a40 100644 (file)
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -123,8 +123,7 @@ int vkms_gem_vmap(struct drm_gem_object *obj);
 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_set_crc_source(struct drm_crtc *crtc, const char *src_name);
 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);

diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f2dd180a867abda8cafb463411d33ba249cf44c8..b21437bc95bf3169319b5baee2b356d149ee6e49 100644 (file)
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -744,8 +744,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:
         *