[PATCH 08/10] drm/crc: Cleanup crtc_crc_open function
Mahesh Kumar
mahesh1.kumar at intel.com
Thu Jul 12 08:36:33 UTC 2018
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
Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
Cc: dri-devel at lists.freedesktop.org
Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Acked-by: Leo Li <sunpeng.li at 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 | 61 ++++++++++------------
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, 37 insertions(+), 52 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 940b76a1ab71..7386391636e3 100644
--- 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 29b054de9e13..814f6f3e1595 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2155,8 +2155,7 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
/* intel_pipe_crc.c */
#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 83f9ade0cd81..f3c9010e332a 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -583,8 +583,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];
@@ -623,7 +622,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 7124918372f8..72db1834dd50 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -808,8 +808,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 drm_modeset_acquire_ctx ctx;
struct drm_crtc_state *crtc_state;
@@ -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 69886025e628..1013d6596408 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -682,8 +682,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:
*
--
2.16.2
More information about the dri-devel
mailing list