Re: [PATCH 09/10] drm/crc: Cleanup crtc_crc_open function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Cc: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>


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@xxxxxxxxx>
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
---
  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:
  	 *

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux