| Message ID | 20180702110729.8171-9-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2018-07-02 07:07 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 inputs, values_cnt we > already gets as part of verify_crc_source. > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > Cc: dri-devel@lists.freedesktop.org > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Acked-by: Leo Li <sunpeng.li@amd.com> > --- > 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 e43ed064dc46..54056d180003 100644 > --- 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 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 f4d76528d24c..739a813b4b09 100644 > --- a/drivers/gpu/drm/drm_debugfs_crc.c > +++ b/drivers/gpu/drm/drm_debugfs_crc.c > @@ -124,11 +124,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); > > @@ -193,12 +191,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) > @@ -210,30 +211,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 > @@ -250,7 +243,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); > @@ -262,9 +255,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); > @@ -370,7 +362,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 99e3b6477d39..7fe0341bcc27 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2154,8 +2154,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 1dffc346f1bc..27d560f7a817 100644 > --- 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 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 77e91b15ddb4..657372306623 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) > @@ -1152,7 +1150,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 ffaec138aeee..81780325f36f 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: > * >
Hi Mahesh, Thank you for the patch. On Monday, 2 July 2018 14:07:27 EEST 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 inputs, values_cnt we > already gets as part of verify_crc_source. > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > Cc: dri-devel@lists.freedesktop.org > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > 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(-) [snip] > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c > b/drivers/gpu/drm/drm_debugfs_crc.c index f4d76528d24c..739a813b4b09 100644 > --- a/drivers/gpu/drm/drm_debugfs_crc.c > +++ b/drivers/gpu/drm/drm_debugfs_crc.c > @@ -124,11 +124,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); > > @@ -193,12 +191,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) > @@ -210,30 +211,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; > } If you moved allocation before the !crc->opened check, you could group the two code blocks protected by the crc->lock. > 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 > @@ -250,7 +243,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); > @@ -262,9 +255,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); > @@ -370,7 +362,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); [snip]
Hi, Thanks for the review. On 7/10/2018 5:19 PM, Laurent Pinchart wrote: > Hi Mahesh, > > Thank you for the patch. > > On Monday, 2 July 2018 14:07:27 EEST 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 inputs, values_cnt we >> already gets as part of verify_crc_source. >> >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> >> Cc: dri-devel@lists.freedesktop.org >> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> 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(-) > [snip] > >> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c >> b/drivers/gpu/drm/drm_debugfs_crc.c index f4d76528d24c..739a813b4b09 100644 >> --- a/drivers/gpu/drm/drm_debugfs_crc.c >> +++ b/drivers/gpu/drm/drm_debugfs_crc.c >> @@ -124,11 +124,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); >> >> @@ -193,12 +191,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) >> @@ -210,30 +211,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; >> } > If you moved allocation before the !crc->opened check, you could group the two > code blocks protected by the crc->lock. agree, will update in next version. -Mahesh >> 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 >> @@ -250,7 +243,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); >> @@ -262,9 +255,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); >> @@ -370,7 +362,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); > [snip] >
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 e43ed064dc46..54056d180003 100644 --- 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 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 f4d76528d24c..739a813b4b09 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -124,11 +124,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); @@ -193,12 +191,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) @@ -210,30 +211,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 @@ -250,7 +243,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); @@ -262,9 +255,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); @@ -370,7 +362,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 99e3b6477d39..7fe0341bcc27 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2154,8 +2154,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 1dffc346f1bc..27d560f7a817 100644 --- 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 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 77e91b15ddb4..657372306623 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) @@ -1152,7 +1150,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 ffaec138aeee..81780325f36f 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: *