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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
