Re: [PATCH 2/5] drm/msm/dsi: add implementation for helper functions

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

 



Hi Jordan,
Thanks for the review. Will incorporate the suggested changes in v2.

On 03/12/2018 08:43 PM, Jordan Crouse wrote:
On Mon, Mar 12, 2018 at 06:53:11PM +0530, Sibi S wrote:
Add dsi host helper function implementation for DSI v2
and DSI 6G 1.x controllers

Signed-off-by: Sibi S <sibis@xxxxxxxxxxxxxx>
---
  drivers/gpu/drm/msm/dsi/dsi.h      |  15 +++
  drivers/gpu/drm/msm/dsi/dsi_cfg.c  |  44 +++++--
  drivers/gpu/drm/msm/dsi/dsi_host.c | 250 ++++++++++++++++++++++++++++++++++++-
  3 files changed, 298 insertions(+), 11 deletions(-)

<snip>
  static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host)
  {
  	struct drm_display_mode *mode = msm_host->mode;
@@ -1008,6 +1161,59 @@ static void dsi_wait4video_eng_busy(struct msm_dsi_host *msm_host)
  	}
  }
+int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size)
+{
+	struct drm_device *dev = msm_host->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	int ret;
+	uint64_t iova;
+
+	msm_host->tx_gem_obj = msm_gem_new(dev, size, MSM_BO_UNCACHED);
+	if (IS_ERR(msm_host->tx_gem_obj)) {
+		ret = PTR_ERR(msm_host->tx_gem_obj);
+		pr_err("%s: failed to allocate gem, %d\n",
+			__func__, ret);
+		msm_host->tx_gem_obj = NULL;
+		return ret;
+	}
+
+	ret = msm_gem_get_iova(msm_host->tx_gem_obj,
+			priv->kms->aspace, &iova);
+	mutex_unlock(&dev->struct_mutex);
+	if (ret) {
+		pr_err("%s: failed to get iova, %d\n", __func__, ret);
+		return ret;
+	}
+
+	if (iova & 0x07) {
+		pr_err("%s: buf NOT 8 bytes aligned\n", __func__);
+		return -EINVAL;
+	}

This is impossible - new allocations will always be page aligned.


Its always good to review and remove older code paths :).
Sure will remove this check.

+	msm_host->tx_size = msm_host->tx_gem_obj->size;
+
+	return 0;
+}
+
+int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size)
+{
+	struct drm_device *dev = msm_host->dev;
+	int ret;
+
+	msm_host->tx_buf = dma_alloc_coherent(dev->dev, size,
+					&msm_host->tx_buf_paddr, GFP_KERNEL);
+	if (!msm_host->tx_buf) {
+		ret = -ENOMEM;
+		pr_err("%s: failed to allocate tx buf, %d\n",
+			__func__, ret);

You don't need to print ret here, it isn't a mystery what it is.  In fact, you
probably don't need to print anything here at all because dma_alloc_coherent
should be pretty noisy when it fails.

+		return ret;

This can just be return -ENOMEM and you can lose 'ret'.


yep makes sense, will replace it.

+	}
+
+	msm_host->tx_size = size;
+
+	return 0;
+}
+
  /* dsi_cmd */
  static int dsi_tx_buf_alloc(struct msm_dsi_host *msm_host, int size)
  {
@@ -1072,6 +1278,21 @@ static void dsi_tx_buf_free(struct msm_dsi_host *msm_host)
  			msm_host->tx_buf_paddr);
  }
+void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host)
+{
+	return msm_gem_get_vaddr(msm_host->tx_gem_obj);
+}
+
+void *dsi_tx_buf_get_v2(struct msm_dsi_host *msm_host)
+{
+	return msm_host->tx_buf;
+}
+
+void dsi_tx_buf_put_6g(struct msm_dsi_host *msm_host)
+{
+	msm_gem_put_vaddr(msm_host->tx_gem_obj);
+}
+
  /*
   * prepare cmd buffer to be txed
   */
@@ -1173,6 +1394,31 @@ static int dsi_long_read_resp(u8 *buf, const struct mipi_dsi_msg *msg)
  	return msg->rx_len;
  }
+int dsi_dma_base_get_6g(struct msm_dsi_host *msm_host, uint64_t *dma_base)
+{
+	struct drm_device *dev = msm_host->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	uint64_t **iova;
+	int ret;
+
+	if (!dma_base)
+		return -EINVAL;
+
+	iova = &dma_base;

This is a convoluted way of passing in the pointer and I doubt even the compiler
can see through it.

+	ret = msm_gem_get_iova(msm_host->tx_gem_obj,
+				priv->kms->aspace, *iova);

ret = msm_gem_get_iova(msm_host->tx_gem_obj, priv->kms->aspace, dma_base);

Easy, safe effective

+	return ret;

If you put a return on the front of the msm_gem_get_iova you can eliminate the
need for 'ret'.


ok will do just that.

Jordan


--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux