Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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

authored by

Mahesh Kumar and committed by
Rodrigo Vivi
c0811a7d af697933

+43 -61
+1 -2
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
··· 258 258 259 259 /* amdgpu_dm_crc.c */ 260 260 #ifdef CONFIG_DEBUG_FS 261 - int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name, 262 - size_t *values_cnt); 261 + int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name); 263 262 int amdgpu_dm_crtc_verify_crc_source(struct drm_crtc *crtc, 264 263 const char *src_name, 265 264 size_t *values_cnt);
+1 -3
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
··· 62 62 return 0; 63 63 } 64 64 65 - int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name, 66 - size_t *values_cnt) 65 + int amdgpu_dm_crtc_set_crc_source(struct drm_crtc *crtc, const char *src_name) 67 66 { 68 67 struct dm_crtc_state *crtc_state = to_dm_crtc_state(crtc->state); 69 68 struct dc_stream_state *stream_state = crtc_state->stream; ··· 98 99 return -EINVAL; 99 100 } 100 101 101 - *values_cnt = 3; 102 102 /* Reset crc_skipped on dm state */ 103 103 crtc_state->crc_skip_count = 0; 104 104 return 0;
+33 -36
drivers/gpu/drm/drm_debugfs_crc.c
··· 127 127 if (source[len] == '\n') 128 128 source[len] = '\0'; 129 129 130 - if (crtc->funcs->verify_crc_source) { 131 - ret = crtc->funcs->verify_crc_source(crtc, source, &values_cnt); 132 - if (ret) 133 - return ret; 134 - } 130 + ret = crtc->funcs->verify_crc_source(crtc, source, &values_cnt); 131 + if (ret) 132 + return ret; 135 133 136 134 spin_lock_irq(&crc->lock); 137 135 ··· 195 197 return ret; 196 198 } 197 199 198 - spin_lock_irq(&crc->lock); 199 - if (!crc->opened) 200 - crc->opened = true; 201 - else 202 - ret = -EBUSY; 203 - spin_unlock_irq(&crc->lock); 204 - 200 + ret = crtc->funcs->verify_crc_source(crtc, crc->source, &values_cnt); 205 201 if (ret) 206 202 return ret; 207 203 208 - ret = crtc->funcs->set_crc_source(crtc, crc->source, &values_cnt); 204 + if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) 205 + return -EINVAL; 206 + 207 + if (WARN_ON(values_cnt == 0)) 208 + return -EINVAL; 209 + 210 + entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL); 211 + if (!entries) 212 + return -ENOMEM; 213 + 214 + spin_lock_irq(&crc->lock); 215 + if (!crc->opened) { 216 + crc->opened = true; 217 + crc->entries = entries; 218 + crc->values_cnt = values_cnt; 219 + } else { 220 + ret = -EBUSY; 221 + } 222 + spin_unlock_irq(&crc->lock); 223 + 224 + if (ret) { 225 + kfree(entries); 226 + return ret; 227 + } 228 + 229 + ret = crtc->funcs->set_crc_source(crtc, crc->source); 209 230 if (ret) 210 231 goto err; 211 232 212 - if (WARN_ON(values_cnt > DRM_MAX_CRC_NR)) { 213 - ret = -EINVAL; 214 - goto err_disable; 215 - } 216 - 217 - if (WARN_ON(values_cnt == 0)) { 218 - ret = -EINVAL; 219 - goto err_disable; 220 - } 221 - 222 - entries = kcalloc(DRM_CRC_ENTRIES_NR, sizeof(*entries), GFP_KERNEL); 223 - if (!entries) { 224 - ret = -ENOMEM; 225 - goto err_disable; 226 - } 227 - 228 233 spin_lock_irq(&crc->lock); 229 - crc->entries = entries; 230 - crc->values_cnt = values_cnt; 231 - 232 234 /* 233 235 * Only return once we got a first frame, so userspace doesn't have to 234 236 * guess when this particular piece of HW will be ready to start ··· 245 247 return 0; 246 248 247 249 err_disable: 248 - crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); 250 + crtc->funcs->set_crc_source(crtc, NULL); 249 251 err: 250 252 spin_lock_irq(&crc->lock); 251 253 crtc_crc_cleanup(crc); ··· 257 259 { 258 260 struct drm_crtc *crtc = filep->f_inode->i_private; 259 261 struct drm_crtc_crc *crc = &crtc->crc; 260 - size_t values_cnt; 261 262 262 - crtc->funcs->set_crc_source(crtc, NULL, &values_cnt); 263 + crtc->funcs->set_crc_source(crtc, NULL); 263 264 264 265 spin_lock_irq(&crc->lock); 265 266 crtc_crc_cleanup(crc); ··· 364 367 { 365 368 struct dentry *crc_ent, *ent; 366 369 367 - if (!crtc->funcs->set_crc_source) 370 + if (!crtc->funcs->set_crc_source || !crtc->funcs->verify_crc_source) 368 371 return 0; 369 372 370 373 crc_ent = debugfs_create_dir("crc", crtc->debugfs_entry);
+1 -2
drivers/gpu/drm/i915/intel_drv.h
··· 2151 2151 /* intel_pipe_crc.c */ 2152 2152 int intel_pipe_crc_create(struct drm_minor *minor); 2153 2153 #ifdef CONFIG_DEBUG_FS 2154 - int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, 2155 - size_t *values_cnt); 2154 + int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name); 2156 2155 int intel_crtc_verify_crc_source(struct drm_crtc *crtc, 2157 2156 const char *source_name, size_t *values_cnt); 2158 2157 const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
+1 -3
drivers/gpu/drm/i915/intel_pipe_crc.c
··· 1028 1028 return -EINVAL; 1029 1029 } 1030 1030 1031 - int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name, 1032 - size_t *values_cnt) 1031 + int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name) 1033 1032 { 1034 1033 struct drm_i915_private *dev_priv = to_i915(crtc->dev); 1035 1034 struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[crtc->index]; ··· 1067 1068 } 1068 1069 1069 1070 pipe_crc->skipped = 0; 1070 - *values_cnt = 5; 1071 1071 1072 1072 out: 1073 1073 intel_display_power_put(dev_priv, power_domain);
+1 -3
drivers/gpu/drm/rcar-du/rcar_du_crtc.c
··· 887 887 } 888 888 889 889 static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc, 890 - const char *source_name, 891 - size_t *values_cnt) 890 + const char *source_name) 892 891 { 893 892 struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc); 894 893 struct drm_modeset_acquire_ctx ctx; ··· 902 903 return ret; 903 904 904 905 index = ret; 905 - *values_cnt = 1; 906 906 907 907 /* Perform an atomic commit to set the CRC source. */ 908 908 drm_modeset_acquire_init(&ctx, 0);
+2 -4
drivers/gpu/drm/rockchip/rockchip_drm_vop.c
··· 1111 1111 } 1112 1112 1113 1113 static int vop_crtc_set_crc_source(struct drm_crtc *crtc, 1114 - const char *source_name, size_t *values_cnt) 1114 + const char *source_name) 1115 1115 { 1116 1116 struct vop *vop = to_vop(crtc); 1117 1117 struct drm_connector *connector; ··· 1120 1120 connector = vop_get_edp_connector(vop); 1121 1121 if (!connector) 1122 1122 return -EINVAL; 1123 - 1124 - *values_cnt = 3; 1125 1123 1126 1124 if (source_name && strcmp(source_name, "auto") == 0) 1127 1125 ret = analogix_dp_start_crc(connector); ··· 1144 1146 1145 1147 #else 1146 1148 static int vop_crtc_set_crc_source(struct drm_crtc *crtc, 1147 - const char *source_name, size_t *values_cnt) 1149 + const char *source_name) 1148 1150 { 1149 1151 return -ENODEV; 1150 1152 }
+1 -4
drivers/gpu/drm/vkms/vkms_crc.c
··· 101 101 return 0; 102 102 } 103 103 104 - int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name, 105 - size_t *values_cnt) 104 + int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name) 106 105 { 107 106 struct vkms_output *out = drm_crtc_to_vkms_output(crtc); 108 107 bool enabled = false; ··· 109 110 int ret = 0; 110 111 111 112 ret = vkms_crc_parse_source(src_name, &enabled); 112 - 113 - *values_cnt = 1; 114 113 115 114 /* make sure nothing is scheduled on crtc workq */ 116 115 flush_workqueue(out->crc_workq);
+1 -2
drivers/gpu/drm/vkms/vkms_drv.h
··· 123 123 void vkms_gem_vunmap(struct drm_gem_object *obj); 124 124 125 125 /* CRC Support */ 126 - int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name, 127 - size_t *values_cnt); 126 + int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name); 128 127 int vkms_verify_crc_source(struct drm_crtc *crtc, const char *source_name, 129 128 size_t *values_cnt); 130 129 void vkms_crc_work_handle(struct work_struct *work);
+1 -2
include/drm/drm_crtc.h
··· 744 744 * 745 745 * 0 on success or a negative error code on failure. 746 746 */ 747 - int (*set_crc_source)(struct drm_crtc *crtc, const char *source, 748 - size_t *values_cnt); 747 + int (*set_crc_source)(struct drm_crtc *crtc, const char *source); 749 748 /** 750 749 * @verify_crc_source: 751 750 *