dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Add OCMEM support
@ 2015-09-28 18:51 Rob Clark
  2015-09-28 18:51 ` [PATCH 1/4] qcom-scm: add ocmem support Rob Clark
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Rob Clark @ 2015-09-28 18:51 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm; +Cc: Stephen Boyd, Bjorn Andersson

For devices such as 8x74 and 8084, which have a separate OCMEM block
used by the GPU (and some other devices), rather than an internal GMEM
block inside the GPU, we need a driver to power up and configure OCMEM
in order to get the GPU working.

This patchset implements a vastly simplified version of the downstream
vendor kernel's OCMEM driver.  Currently it is just enough to enable
the GPU.  But we can worry about other OCMEM users when they have up-
stream drivers.

The first patch adds support for the necessary scm interfaces, for the
parts of the OCMEM configuration that must be done from secure mode.
The second patch can be dropped, as so far this doesn't seem needed for
the GPU (I'm just sending the patch now so it can be found later if it
turns out to be needed).

The remaining two patches add the OCMEM driver inside drm/msm.  If we
eventually have other upstream OCMEM users, this would need to be split
out.  But that should not effect DT bindings, etc, so that is something
that can easily be done later when the need arises.

Rob Clark (4):
  qcom-scm: add ocmem support
  WIP: qcom-scm: add ocmem dump support
  drm/msm: update generated headers
  drm/msm: add OCMEM driver

 drivers/firmware/qcom_scm-32.c                 |  89 ++++++
 drivers/firmware/qcom_scm-64.c                 |  26 ++
 drivers/firmware/qcom_scm.c                    | 160 ++++++++++
 drivers/firmware/qcom_scm.h                    |  17 ++
 drivers/gpu/drm/msm/Makefile                   |   3 +-
 drivers/gpu/drm/msm/adreno/a2xx.xml.h          |   9 +-
 drivers/gpu/drm/msm/adreno/a3xx.xml.h          |  27 +-
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c          |  17 +-
 drivers/gpu/drm/msm/adreno/a4xx.xml.h          |  15 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c          |  19 +-
 drivers/gpu/drm/msm/adreno/adreno_common.xml.h |  13 +-
 drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h    |   9 +-
 drivers/gpu/drm/msm/dsi/dsi.xml.h              |   4 +-
 drivers/gpu/drm/msm/dsi/mmss_cc.xml.h          |   4 +-
 drivers/gpu/drm/msm/dsi/sfpb.xml.h             |   4 +-
 drivers/gpu/drm/msm/edp/edp.xml.h              |   4 +-
 drivers/gpu/drm/msm/hdmi/hdmi.xml.h            |   4 +-
 drivers/gpu/drm/msm/hdmi/qfprom.xml.h          |   4 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4.xml.h        |   4 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h        |  82 ++++-
 drivers/gpu/drm/msm/mdp/mdp_common.xml.h       |  11 +-
 drivers/gpu/drm/msm/msm_drv.c                  |   2 +
 drivers/gpu/drm/msm/msm_gpu.h                  |   3 +
 drivers/gpu/drm/msm/ocmem/ocmem.c              | 396 +++++++++++++++++++++++++
 drivers/gpu/drm/msm/ocmem/ocmem.h              |  46 +++
 drivers/gpu/drm/msm/ocmem/ocmem.xml.h          | 113 +++++++
 include/linux/qcom_scm.h                       |  14 +
 27 files changed, 1032 insertions(+), 67 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.c
 create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.h
 create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.xml.h

-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] qcom-scm: add ocmem support
  2015-09-28 18:51 [PATCH 0/4] Add OCMEM support Rob Clark
@ 2015-09-28 18:51 ` Rob Clark
  2015-09-28 20:51   ` Stephen Boyd
  2015-09-28 18:51 ` [PATCH 2/4] WIP: qcom-scm: add ocmem dump support Rob Clark
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2015-09-28 18:51 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm; +Cc: Stephen Boyd, Bjorn Andersson

Add interfaces needed for configuring OCMEM.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/firmware/qcom_scm-32.c |  57 ++++++++++++++++++++++
 drivers/firmware/qcom_scm-64.c |  16 +++++++
 drivers/firmware/qcom_scm.c    | 106 +++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    |  13 +++++
 include/linux/qcom_scm.h       |  10 ++++
 5 files changed, 202 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index e9c306b..656d8fe 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -500,6 +500,63 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
 }
 
+int __qcom_scm_ocmem_secure_cfg(unsigned sec_id)
+{
+	int ret, scm_ret = 0;
+	struct msm_scm_sec_cfg {
+		unsigned int id;
+		unsigned int spare;
+	} cfg;
+
+	cfg.id = sec_id;
+
+
+	ret = qcom_scm_call(QCOM_SCM_OCMEM_SECURE_SVC, QCOM_SCM_OCMEM_SECURE_CFG,
+			&cfg, sizeof(cfg), &scm_ret, sizeof(scm_ret));
+
+	if (ret || scm_ret) {
+		pr_err("ocmem: Failed to enable secure programming\n");
+		return ret ? ret : -EINVAL;
+	}
+
+	return 0;
+}
+
+int __qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
+		uint32_t mode)
+{
+	struct ocmem_tz_lock {
+		u32 id;
+		u32 offset;
+		u32 size;
+		u32 mode;
+	} request;
+
+	request.id = id;
+	request.offset = offset;
+	request.size = size;
+	request.mode = mode;
+
+	return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_LOCK_CMD,
+			&request, sizeof(request), NULL, 0);
+}
+
+int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
+{
+	struct ocmem_tz_unlock {
+		u32 id;
+		u32 offset;
+		u32 size;
+	} request;
+
+	request.id = id;
+	request.offset = offset;
+	request.size = size;
+
+	return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_UNLOCK_CMD,
+			&request, sizeof(request), NULL, 0);
+}
+
 bool __qcom_scm_pas_supported(u32 peripheral)
 {
 	__le32 out;
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index e64fd92..ef5c59e 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -62,6 +62,22 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 	return -ENOTSUPP;
 }
 
+int __qcom_scm_ocmem_secure_cfg(unsigned sec_id)
+{
+	return -ENOTSUPP;
+}
+
+int __qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
+		uint32_t mode)
+{
+	return -ENOTSUPP;
+}
+
+int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
+{
+	return -ENOTSUPP;
+}
+
 bool __qcom_scm_pas_supported(u32 peripheral)
 {
 	return false;
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 118df0a..59b1007 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -154,6 +154,112 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
 EXPORT_SYMBOL(qcom_scm_hdcp_req);
 
 /**
+ * qcom_scm_ocmem_secure_available() - Check if secure environment supports
+ * OCMEM.
+ *
+ * Return true if OCMEM secure interface is supported, false if not.
+ */
+bool qcom_scm_ocmem_secure_available(void)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		goto clk_err;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SECURE_SVC,
+			QCOM_SCM_OCMEM_SECURE_CFG);
+
+	qcom_scm_clk_disable();
+
+clk_err:
+	return (ret > 0) ? true : false;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_secure_available);
+
+/**
+ * qcom_scm_ocmem_secure_cfg() - call OCMEM secure cfg interface
+ */
+int qcom_scm_ocmem_secure_cfg(unsigned sec_id)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_ocmem_secure_cfg(sec_id);
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_secure_cfg);
+
+/**
+ * qcom_scm_ocmem_lock_available() - is OCMEM lock/unlock interface available
+ */
+bool qcom_scm_ocmem_lock_available(void)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		goto clk_err;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SVC,
+			QCOM_SCM_OCMEM_LOCK_CMD);
+
+	qcom_scm_clk_disable();
+
+clk_err:
+	return (ret > 0) ? true : false;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_lock_available);
+
+/**
+ * qcom_scm_ocmem_lock() - call OCMEM lock interface to assign an OCMEM
+ * region to the specified initiator
+ *
+ * @id:     tz initiator id
+ * @offset: OCMEM offset
+ * @size:   OCMEM size
+ * @mode:   access mode (WIDE/NARROW)
+ */
+int qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
+		uint32_t mode)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_ocmem_lock(id, offset, size, mode);
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_lock);
+
+/**
+ * qcom_scm_ocmem_unlock() - call OCMEM unlock interface to release an OCMEM
+ * region from the specified initiator
+ *
+ * @id:     tz initiator id
+ * @offset: OCMEM offset
+ * @size:   OCMEM size
+ */
+int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_ocmem_unlock(id, offset, size);
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
+
+/**
  * qcom_scm_pas_supported() - Check if the peripheral authentication service is
  *			      available for the given peripherial
  * @peripheral:	peripheral id
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 220d19c..e01656f3 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -36,6 +36,19 @@ extern int __qcom_scm_is_call_available(u32 svc_id, u32 cmd_id);
 extern int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+#define QCOM_SCM_OCMEM_SECURE_SVC		0xc
+#define QCOM_SCM_OCMEM_SECURE_CFG		0x2
+
+extern int __qcom_scm_ocmem_secure_cfg(unsigned sec_id);
+
+#define QCOM_SCM_OCMEM_SVC			0xf
+#define QCOM_SCM_OCMEM_LOCK_CMD		0x1
+#define QCOM_SCM_OCMEM_UNLOCK_CMD		0x2
+
+extern int __qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
+		uint32_t mode);
+extern int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size);
+
 #define QCOM_SCM_SVC_PIL		0x2
 #define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
 #define QCOM_SCM_PAS_MEM_SETUP_CMD	0x2
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 46d41e4..a934457 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -23,10 +23,20 @@ struct qcom_scm_hdcp_req {
 	u32 val;
 };
 
+extern bool qcom_scm_is_available(void);
+
 extern bool qcom_scm_hdcp_available(void);
 extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 		u32 *resp);
 
+extern bool qcom_scm_ocmem_secure_available(void);
+extern int qcom_scm_ocmem_secure_cfg(unsigned sec_id);
+
+extern bool qcom_scm_ocmem_lock_available(void);
+extern int qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
+		uint32_t mode);
+extern int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size);
+
 extern bool qcom_scm_pas_supported(u32 peripheral);
 extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
 extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
-- 
2.4.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] WIP: qcom-scm: add ocmem dump support
  2015-09-28 18:51 [PATCH 0/4] Add OCMEM support Rob Clark
  2015-09-28 18:51 ` [PATCH 1/4] qcom-scm: add ocmem support Rob Clark
@ 2015-09-28 18:51 ` Rob Clark
  2015-09-28 18:51 ` [PATCH 3/4] drm/msm: update generated headers Rob Clark
  2015-09-28 18:51 ` [PATCH 4/4] drm/msm: add OCMEM driver Rob Clark
  3 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-09-28 18:51 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm; +Cc: Stephen Boyd, Bjorn Andersson, Rob Clark

Seems so far not to be required, at least for gpu.  Just stuffing it in
a patch since I wrote the code and someone might want to resurrect this
at some later time.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/firmware/qcom_scm-32.c | 32 +++++++++++++++++++++++++
 drivers/firmware/qcom_scm-64.c | 10 ++++++++
 drivers/firmware/qcom_scm.c    | 54 ++++++++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h    |  4 ++++
 include/linux/qcom_scm.h       |  4 ++++
 5 files changed, 104 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 656d8fe..287041a 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -557,6 +557,38 @@ int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
 			&request, sizeof(request), NULL, 0);
 }
 
+int __qcom_scm_ocmem_enable_dump(uint32_t id, uint32_t offset, uint32_t size)
+{
+	struct ocmem_tz_en_dump {
+		u32 id;
+		u32 offset;
+		u32 size;
+	} request;
+
+	request.id = id;
+	request.offset = offset;
+	request.size = size;
+
+	return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_ENABLE_DUMP_CMD,
+			&request, sizeof(request), NULL, 0);
+}
+
+int __qcom_scm_ocmem_disable_dump(uint32_t id, uint32_t offset, uint32_t size)
+{
+	struct ocmem_tz_dis_dump {
+		u32 id;
+		u32 offset;
+		u32 size;
+	} request;
+
+	request.id = id;
+	request.offset = offset;
+	request.size = size;
+
+	return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_DISABLE_DUMP_CMD,
+			&request, sizeof(request), NULL, 0);
+}
+
 bool __qcom_scm_pas_supported(u32 peripheral)
 {
 	__le32 out;
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index ef5c59e..fc02828 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -78,6 +78,16 @@ int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
 	return -ENOTSUPP;
 }
 
+int __qcom_scm_ocmem_enable_dump(uint32_t id, uint32_t offset, uint32_t size)
+{
+	return -ENOTSUPP;
+}
+
+int __qcom_scm_ocmem_disable_dump(uint32_t id, uint32_t offset, uint32_t size)
+{
+	return -ENOTSUPP;
+}
+
 bool __qcom_scm_pas_supported(u32 peripheral)
 {
 	return false;
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 59b1007..b15b0d8 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -260,6 +260,60 @@ int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
 EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
 
 /**
+ *
+ */
+bool qcom_scm_ocmem_dump_available(void)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		goto clk_err;
+
+	ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SVC,
+			QCOM_SCM_OCMEM_ENABLE_DUMP_CMD);
+
+	qcom_scm_clk_disable();
+
+clk_err:
+	return (ret > 0) ? true : false;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_dump_available);
+
+/**
+ *
+ */
+int qcom_scm_ocmem_enable_dump(uint32_t id, uint32_t offset, uint32_t size)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_ocmem_enable_dump(id, offset, size);
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_enable_dump);
+
+/**
+ *
+ */
+int qcom_scm_ocmem_disable_dump(uint32_t id, uint32_t offset, uint32_t size)
+{
+	int ret = qcom_scm_clk_enable();
+
+	if (ret)
+		return ret;
+
+	ret = __qcom_scm_ocmem_disable_dump(id, offset, size);
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL(qcom_scm_ocmem_disable_dump);
+
+/**
  * qcom_scm_pas_supported() - Check if the peripheral authentication service is
  *			      available for the given peripherial
  * @peripheral:	peripheral id
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index e01656f3..a090236 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -44,10 +44,14 @@ extern int __qcom_scm_ocmem_secure_cfg(unsigned sec_id);
 #define QCOM_SCM_OCMEM_SVC			0xf
 #define QCOM_SCM_OCMEM_LOCK_CMD		0x1
 #define QCOM_SCM_OCMEM_UNLOCK_CMD		0x2
+#define QCOM_SCM_OCMEM_ENABLE_DUMP_CMD		0x3
+#define QCOM_SCM_OCMEM_DISABLE_DUMP_CMD	0x4
 
 extern int __qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
 		uint32_t mode);
 extern int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size);
+extern int __qcom_scm_ocmem_enable_dump(uint32_t id, uint32_t offset, uint32_t size);
+extern int __qcom_scm_ocmem_disable_dump(uint32_t id, uint32_t offset, uint32_t size);
 
 #define QCOM_SCM_SVC_PIL		0x2
 #define QCOM_SCM_PAS_INIT_IMAGE_CMD	0x1
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index a934457..bdbbbdf 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -37,6 +37,10 @@ extern int qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
 		uint32_t mode);
 extern int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size);
 
+extern bool qcom_scm_ocmem_dump_available(void);
+extern int qcom_scm_ocmem_enable_dump(uint32_t id, uint32_t offset, uint32_t size);
+extern int qcom_scm_ocmem_disable_dump(uint32_t id, uint32_t offset, uint32_t size);
+
 extern bool qcom_scm_pas_supported(u32 peripheral);
 extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size);
 extern int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] drm/msm: update generated headers
  2015-09-28 18:51 [PATCH 0/4] Add OCMEM support Rob Clark
  2015-09-28 18:51 ` [PATCH 1/4] qcom-scm: add ocmem support Rob Clark
  2015-09-28 18:51 ` [PATCH 2/4] WIP: qcom-scm: add ocmem dump support Rob Clark
@ 2015-09-28 18:51 ` Rob Clark
  2015-09-28 18:51 ` [PATCH 4/4] drm/msm: add OCMEM driver Rob Clark
  3 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-09-28 18:51 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm; +Cc: Stephen Boyd, Bjorn Andersson, Rob Clark

Update generated headers to pull in OCMEM register definitions.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/adreno/a2xx.xml.h          |   9 +-
 drivers/gpu/drm/msm/adreno/a3xx.xml.h          |  27 ++++--
 drivers/gpu/drm/msm/adreno/a4xx.xml.h          |  15 ++--
 drivers/gpu/drm/msm/adreno/adreno_common.xml.h |  13 ++-
 drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h    |   9 +-
 drivers/gpu/drm/msm/dsi/dsi.xml.h              |   4 +-
 drivers/gpu/drm/msm/dsi/mmss_cc.xml.h          |   4 +-
 drivers/gpu/drm/msm/dsi/sfpb.xml.h             |   4 +-
 drivers/gpu/drm/msm/edp/edp.xml.h              |   4 +-
 drivers/gpu/drm/msm/hdmi/hdmi.xml.h            |   4 +-
 drivers/gpu/drm/msm/hdmi/qfprom.xml.h          |   4 +-
 drivers/gpu/drm/msm/mdp/mdp4/mdp4.xml.h        |   4 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h        |  82 +++++++++++++++++-
 drivers/gpu/drm/msm/mdp/mdp_common.xml.h       |  11 ++-
 drivers/gpu/drm/msm/ocmem/ocmem.xml.h          | 113 +++++++++++++++++++++++++
 15 files changed, 267 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.xml.h

diff --git a/drivers/gpu/drm/msm/adreno/a2xx.xml.h b/drivers/gpu/drm/msm/adreno/a2xx.xml.h
index 0261f0d..9e2aceb 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx.xml.h
+++ b/drivers/gpu/drm/msm/adreno/a2xx.xml.h
@@ -8,13 +8,14 @@ http://github.com/freedreno/envytools/
 git clone https://github.com/freedreno/envytools.git
 
 The rules-ng-ng source files this header was generated from are:
-- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    364 bytes, from 2015-05-20 20:03:07)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    398 bytes, from 2015-09-24 17:25:31)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml  (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/a2xx.xml          (  32901 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10551 bytes, from 2015-05-20 20:03:14)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10755 bytes, from 2015-09-14 20:46:55)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_pm4.xml    (  14968 bytes, from 2015-05-20 20:12:27)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67120 bytes, from 2015-08-14 23:22:03)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63785 bytes, from 2015-08-14 18:27:06)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67771 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63970 bytes, from 2015-09-14 20:50:12)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/ocmem.xml         (   1773 bytes, from 2015-09-24 17:30:00)
 
 Copyright (C) 2013-2015 by the following authors:
 - Rob Clark <robdclark@gmail.com> (robclark)
diff --git a/drivers/gpu/drm/msm/adreno/a3xx.xml.h b/drivers/gpu/drm/msm/adreno/a3xx.xml.h
index 48d1337..97dc1c6 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx.xml.h
+++ b/drivers/gpu/drm/msm/adreno/a3xx.xml.h
@@ -8,13 +8,14 @@ http://github.com/freedreno/envytools/
 git clone https://github.com/freedreno/envytools.git
 
 The rules-ng-ng source files this header was generated from are:
-- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    364 bytes, from 2015-05-20 20:03:07)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    398 bytes, from 2015-09-24 17:25:31)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml  (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/a2xx.xml          (  32901 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10551 bytes, from 2015-05-20 20:03:14)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10755 bytes, from 2015-09-14 20:46:55)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_pm4.xml    (  14968 bytes, from 2015-05-20 20:12:27)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67120 bytes, from 2015-08-14 23:22:03)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63785 bytes, from 2015-08-14 18:27:06)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67771 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63970 bytes, from 2015-09-14 20:50:12)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/ocmem.xml         (   1773 bytes, from 2015-09-24 17:30:00)
 
 Copyright (C) 2013-2015 by the following authors:
 - Rob Clark <robdclark@gmail.com> (robclark)
@@ -280,6 +281,8 @@ enum a3xx_rb_blend_opcode {
 enum a3xx_intp_mode {
 	SMOOTH = 0,
 	FLAT = 1,
+	ZERO = 2,
+	ONE = 3,
 };
 
 enum a3xx_repl_mode {
@@ -680,9 +683,16 @@ static inline uint32_t REG_A3XX_CP_PROTECT_REG(uint32_t i0) { return 0x00000460
 #define A3XX_GRAS_CL_CLIP_CNTL_VP_CLIP_CODE_IGNORE		0x00080000
 #define A3XX_GRAS_CL_CLIP_CNTL_VP_XFORM_DISABLE			0x00100000
 #define A3XX_GRAS_CL_CLIP_CNTL_PERSP_DIVISION_DISABLE		0x00200000
+#define A3XX_GRAS_CL_CLIP_CNTL_ZERO_GB_SCALE_Z			0x00400000
 #define A3XX_GRAS_CL_CLIP_CNTL_ZCOORD				0x00800000
 #define A3XX_GRAS_CL_CLIP_CNTL_WCOORD				0x01000000
 #define A3XX_GRAS_CL_CLIP_CNTL_ZCLIP_DISABLE			0x02000000
+#define A3XX_GRAS_CL_CLIP_CNTL_NUM_USER_CLIP_PLANES__MASK	0x1c000000
+#define A3XX_GRAS_CL_CLIP_CNTL_NUM_USER_CLIP_PLANES__SHIFT	26
+static inline uint32_t A3XX_GRAS_CL_CLIP_CNTL_NUM_USER_CLIP_PLANES(uint32_t val)
+{
+	return ((val) << A3XX_GRAS_CL_CLIP_CNTL_NUM_USER_CLIP_PLANES__SHIFT) & A3XX_GRAS_CL_CLIP_CNTL_NUM_USER_CLIP_PLANES__MASK;
+}
 
 #define REG_A3XX_GRAS_CL_GB_CLIP_ADJ				0x00002044
 #define A3XX_GRAS_CL_GB_CLIP_ADJ_HORZ__MASK			0x000003ff
@@ -773,7 +783,7 @@ static inline uint32_t A3XX_GRAS_SU_POINT_SIZE(float val)
 #define A3XX_GRAS_SU_POLY_OFFSET_SCALE_VAL__SHIFT		0
 static inline uint32_t A3XX_GRAS_SU_POLY_OFFSET_SCALE_VAL(float val)
 {
-	return ((((int32_t)(val * 16384.0))) << A3XX_GRAS_SU_POLY_OFFSET_SCALE_VAL__SHIFT) & A3XX_GRAS_SU_POLY_OFFSET_SCALE_VAL__MASK;
+	return ((((int32_t)(val * 1048576.0))) << A3XX_GRAS_SU_POLY_OFFSET_SCALE_VAL__SHIFT) & A3XX_GRAS_SU_POLY_OFFSET_SCALE_VAL__MASK;
 }
 
 #define REG_A3XX_GRAS_SU_POLY_OFFSET_OFFSET			0x0000206d
@@ -894,6 +904,9 @@ static inline uint32_t A3XX_RB_MODE_CONTROL_MRT(uint32_t val)
 #define A3XX_RB_MODE_CONTROL_PACKER_TIMER_ENABLE		0x00010000
 
 #define REG_A3XX_RB_RENDER_CONTROL				0x000020c1
+#define A3XX_RB_RENDER_CONTROL_DUAL_COLOR_IN_ENABLE		0x00000001
+#define A3XX_RB_RENDER_CONTROL_YUV_IN_ENABLE			0x00000002
+#define A3XX_RB_RENDER_CONTROL_COV_VALUE_INPUT_ENABLE		0x00000004
 #define A3XX_RB_RENDER_CONTROL_FACENESS				0x00000008
 #define A3XX_RB_RENDER_CONTROL_BIN_WIDTH__MASK			0x00000ff0
 #define A3XX_RB_RENDER_CONTROL_BIN_WIDTH__SHIFT			4
@@ -907,6 +920,8 @@ static inline uint32_t A3XX_RB_RENDER_CONTROL_BIN_WIDTH(uint32_t val)
 #define A3XX_RB_RENDER_CONTROL_YCOORD				0x00008000
 #define A3XX_RB_RENDER_CONTROL_ZCOORD				0x00010000
 #define A3XX_RB_RENDER_CONTROL_WCOORD				0x00020000
+#define A3XX_RB_RENDER_CONTROL_I_CLAMP_ENABLE			0x00080000
+#define A3XX_RB_RENDER_CONTROL_COV_VALUE_OUTPUT_ENABLE		0x00100000
 #define A3XX_RB_RENDER_CONTROL_ALPHA_TEST			0x00400000
 #define A3XX_RB_RENDER_CONTROL_ALPHA_TEST_FUNC__MASK		0x07000000
 #define A3XX_RB_RENDER_CONTROL_ALPHA_TEST_FUNC__SHIFT		24
@@ -914,6 +929,8 @@ static inline uint32_t A3XX_RB_RENDER_CONTROL_ALPHA_TEST_FUNC(enum adreno_compar
 {
 	return ((val) << A3XX_RB_RENDER_CONTROL_ALPHA_TEST_FUNC__SHIFT) & A3XX_RB_RENDER_CONTROL_ALPHA_TEST_FUNC__MASK;
 }
+#define A3XX_RB_RENDER_CONTROL_ALPHA_TO_COVERAGE		0x40000000
+#define A3XX_RB_RENDER_CONTROL_ALPHA_TO_ONE			0x80000000
 
 #define REG_A3XX_RB_MSAA_CONTROL				0x000020c2
 #define A3XX_RB_MSAA_CONTROL_DISABLE				0x00000400
diff --git a/drivers/gpu/drm/msm/adreno/a4xx.xml.h b/drivers/gpu/drm/msm/adreno/a4xx.xml.h
index ac55066..99de827 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx.xml.h
+++ b/drivers/gpu/drm/msm/adreno/a4xx.xml.h
@@ -8,13 +8,14 @@ http://github.com/freedreno/envytools/
 git clone https://github.com/freedreno/envytools.git
 
 The rules-ng-ng source files this header was generated from are:
-- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    364 bytes, from 2015-05-20 20:03:07)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    398 bytes, from 2015-09-24 17:25:31)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml  (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/a2xx.xml          (  32901 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10551 bytes, from 2015-05-20 20:03:14)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10755 bytes, from 2015-09-14 20:46:55)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_pm4.xml    (  14968 bytes, from 2015-05-20 20:12:27)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67120 bytes, from 2015-08-14 23:22:03)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63785 bytes, from 2015-08-14 18:27:06)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67771 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63970 bytes, from 2015-09-14 20:50:12)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/ocmem.xml         (   1773 bytes, from 2015-09-24 17:30:00)
 
 Copyright (C) 2013-2015 by the following authors:
 - Rob Clark <robdclark@gmail.com> (robclark)
@@ -162,10 +163,13 @@ enum a4xx_tex_fmt {
 	TFMT4_8_UNORM = 4,
 	TFMT4_8_8_UNORM = 14,
 	TFMT4_8_8_8_8_UNORM = 28,
+	TFMT4_8_SNORM = 5,
 	TFMT4_8_8_SNORM = 15,
 	TFMT4_8_8_8_8_SNORM = 29,
+	TFMT4_8_UINT = 6,
 	TFMT4_8_8_UINT = 16,
 	TFMT4_8_8_8_8_UINT = 30,
+	TFMT4_8_SINT = 7,
 	TFMT4_8_8_SINT = 17,
 	TFMT4_8_8_8_8_SINT = 31,
 	TFMT4_16_UINT = 21,
@@ -246,7 +250,8 @@ enum a4xx_tex_clamp {
 	A4XX_TEX_REPEAT = 0,
 	A4XX_TEX_CLAMP_TO_EDGE = 1,
 	A4XX_TEX_MIRROR_REPEAT = 2,
-	A4XX_TEX_CLAMP_NONE = 3,
+	A4XX_TEX_CLAMP_TO_BORDER = 3,
+	A4XX_TEX_MIRROR_CLAMP = 4,
 };
 
 enum a4xx_tex_aniso {
diff --git a/drivers/gpu/drm/msm/adreno/adreno_common.xml.h b/drivers/gpu/drm/msm/adreno/adreno_common.xml.h
index 399a9e5..c304468 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_common.xml.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_common.xml.h
@@ -8,13 +8,14 @@ http://github.com/freedreno/envytools/
 git clone https://github.com/freedreno/envytools.git
 
 The rules-ng-ng source files this header was generated from are:
-- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    364 bytes, from 2015-05-20 20:03:07)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    398 bytes, from 2015-09-24 17:25:31)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml  (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/a2xx.xml          (  32901 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10551 bytes, from 2015-05-20 20:03:14)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10755 bytes, from 2015-09-14 20:46:55)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_pm4.xml    (  14968 bytes, from 2015-05-20 20:12:27)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67120 bytes, from 2015-08-14 23:22:03)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63785 bytes, from 2015-08-14 18:27:06)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67771 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63970 bytes, from 2015-09-14 20:50:12)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/ocmem.xml         (   1773 bytes, from 2015-09-24 17:30:00)
 
 Copyright (C) 2013-2015 by the following authors:
 - Rob Clark <robdclark@gmail.com> (robclark)
@@ -85,6 +86,10 @@ enum adreno_rb_blend_factor {
 	FACTOR_CONSTANT_ALPHA = 14,
 	FACTOR_ONE_MINUS_CONSTANT_ALPHA = 15,
 	FACTOR_SRC_ALPHA_SATURATE = 16,
+	FACTOR_SRC1_COLOR = 20,
+	FACTOR_ONE_MINUS_SRC1_COLOR = 21,
+	FACTOR_SRC1_ALPHA = 22,
+	FACTOR_ONE_MINUS_SRC1_ALPHA = 23,
 };
 
 enum adreno_rb_surface_endian {
diff --git a/drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h b/drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h
index 41904fe..a22fef5 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_pm4.xml.h
@@ -8,13 +8,14 @@ http://github.com/freedreno/envytools/
 git clone https://github.com/freedreno/envytools.git
 
 The rules-ng-ng source files this header was generated from are:
-- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    364 bytes, from 2015-05-20 20:03:07)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    398 bytes, from 2015-09-24 17:25:31)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml  (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/a2xx.xml          (  32901 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10551 bytes, from 2015-05-20 20:03:14)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10755 bytes, from 2015-09-14 20:46:55)
 - /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_pm4.xml    (  14968 bytes, from 2015-05-20 20:12:27)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67120 bytes, from 2015-08-14 23:22:03)
-- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63785 bytes, from 2015-08-14 18:27:06)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67771 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63970 bytes, from 2015-09-14 20:50:12)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/ocmem.xml         (   1773 bytes, from 2015-09-24 17:30:00)
 
 Copyright (C) 2013-2015 by the following authors:
 - Rob Clark <robdclark@gmail.com> (robclark)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.xml.h b/drivers/gpu/drm/msm/dsi/dsi.xml.h
index 1d2e32f..c208547 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.xml.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/dsi/mmss_cc.xml.h b/drivers/gpu/drm/msm/dsi/mmss_cc.xml.h
index 5de505e..cb830a3 100644
--- a/drivers/gpu/drm/msm/dsi/mmss_cc.xml.h
+++ b/drivers/gpu/drm/msm/dsi/mmss_cc.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/dsi/sfpb.xml.h b/drivers/gpu/drm/msm/dsi/sfpb.xml.h
index 06cbddf..96f7ecb 100644
--- a/drivers/gpu/drm/msm/dsi/sfpb.xml.h
+++ b/drivers/gpu/drm/msm/dsi/sfpb.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/edp/edp.xml.h b/drivers/gpu/drm/msm/edp/edp.xml.h
index bef1d65..fbef985 100644
--- a/drivers/gpu/drm/msm/edp/edp.xml.h
+++ b/drivers/gpu/drm/msm/edp/edp.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
index 0b1b558..84846c4 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/hdmi/qfprom.xml.h b/drivers/gpu/drm/msm/hdmi/qfprom.xml.h
index 2aa23b9..0ae573c 100644
--- a/drivers/gpu/drm/msm/hdmi/qfprom.xml.h
+++ b/drivers/gpu/drm/msm/hdmi/qfprom.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4.xml.h b/drivers/gpu/drm/msm/mdp/mdp4/mdp4.xml.h
index 74b8673..df7e765 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4.xml.h
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h b/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h
index 3469f50..87c3141 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
@@ -895,6 +895,7 @@ static inline uint32_t MDP5_PIPE_SRC_OP_MODE_BWC(enum mdp5_pipe_bwc val)
 #define MDP5_PIPE_SRC_OP_MODE_IGC_ROM_1				0x00040000
 #define MDP5_PIPE_SRC_OP_MODE_DEINTERLACE			0x00400000
 #define MDP5_PIPE_SRC_OP_MODE_DEINTERLACE_ODD			0x00800000
+#define MDP5_PIPE_SRC_OP_MODE_SW_PIX_EXT_OVERRIDE		0x80000000
 
 static inline uint32_t REG_MDP5_PIPE_SRC_CONSTANT_COLOR(enum mdp5_pipe i0) { return 0x0000003c + __offset_PIPE(i0); }
 
@@ -932,6 +933,83 @@ static inline uint32_t MDP5_PIPE_DECIMATION_HORZ(uint32_t val)
 	return ((val) << MDP5_PIPE_DECIMATION_HORZ__SHIFT) & MDP5_PIPE_DECIMATION_HORZ__MASK;
 }
 
+static inline uint32_t __offset_SW_PIX_EXT(enum mdp_component_type idx)
+{
+	switch (idx) {
+		case COMP_0: return 0x00000100;
+		case COMP_1_2: return 0x00000110;
+		case COMP_3: return 0x00000120;
+		default: return INVALID_IDX(idx);
+	}
+}
+static inline uint32_t REG_MDP5_PIPE_SW_PIX_EXT(enum mdp5_pipe i0, enum mdp_component_type i1) { return 0x00000000 + __offset_PIPE(i0) + __offset_SW_PIX_EXT(i1); }
+
+static inline uint32_t REG_MDP5_PIPE_SW_PIX_EXT_LR(enum mdp5_pipe i0, enum mdp_component_type i1) { return 0x00000000 + __offset_PIPE(i0) + __offset_SW_PIX_EXT(i1); }
+#define MDP5_PIPE_SW_PIX_EXT_LR_LEFT_RPT__MASK			0x000000ff
+#define MDP5_PIPE_SW_PIX_EXT_LR_LEFT_RPT__SHIFT			0
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_LR_LEFT_RPT(uint32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_LR_LEFT_RPT__SHIFT) & MDP5_PIPE_SW_PIX_EXT_LR_LEFT_RPT__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_LR_LEFT_OVF__MASK			0x0000ff00
+#define MDP5_PIPE_SW_PIX_EXT_LR_LEFT_OVF__SHIFT			8
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_LR_LEFT_OVF(int32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_LR_LEFT_OVF__SHIFT) & MDP5_PIPE_SW_PIX_EXT_LR_LEFT_OVF__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_RPT__MASK			0x00ff0000
+#define MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_RPT__SHIFT		16
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_RPT(uint32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_RPT__SHIFT) & MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_RPT__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_OVF__MASK			0xff000000
+#define MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_OVF__SHIFT		24
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_OVF(int32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_OVF__SHIFT) & MDP5_PIPE_SW_PIX_EXT_LR_RIGHT_OVF__MASK;
+}
+
+static inline uint32_t REG_MDP5_PIPE_SW_PIX_EXT_TB(enum mdp5_pipe i0, enum mdp_component_type i1) { return 0x00000004 + __offset_PIPE(i0) + __offset_SW_PIX_EXT(i1); }
+#define MDP5_PIPE_SW_PIX_EXT_TB_TOP_RPT__MASK			0x000000ff
+#define MDP5_PIPE_SW_PIX_EXT_TB_TOP_RPT__SHIFT			0
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_TB_TOP_RPT(uint32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_TB_TOP_RPT__SHIFT) & MDP5_PIPE_SW_PIX_EXT_TB_TOP_RPT__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_TB_TOP_OVF__MASK			0x0000ff00
+#define MDP5_PIPE_SW_PIX_EXT_TB_TOP_OVF__SHIFT			8
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_TB_TOP_OVF(int32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_TB_TOP_OVF__SHIFT) & MDP5_PIPE_SW_PIX_EXT_TB_TOP_OVF__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_RPT__MASK		0x00ff0000
+#define MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_RPT__SHIFT		16
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_RPT(uint32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_RPT__SHIFT) & MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_RPT__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_OVF__MASK		0xff000000
+#define MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_OVF__SHIFT		24
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_OVF(int32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_OVF__SHIFT) & MDP5_PIPE_SW_PIX_EXT_TB_BOTTOM_OVF__MASK;
+}
+
+static inline uint32_t REG_MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS(enum mdp5_pipe i0, enum mdp_component_type i1) { return 0x00000008 + __offset_PIPE(i0) + __offset_SW_PIX_EXT(i1); }
+#define MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_LEFT_RIGHT__MASK	0x0000ffff
+#define MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_LEFT_RIGHT__SHIFT	0
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_LEFT_RIGHT(uint32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_LEFT_RIGHT__SHIFT) & MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_LEFT_RIGHT__MASK;
+}
+#define MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_TOP_BOTTOM__MASK	0xffff0000
+#define MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_TOP_BOTTOM__SHIFT	16
+static inline uint32_t MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_TOP_BOTTOM(uint32_t val)
+{
+	return ((val) << MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_TOP_BOTTOM__SHIFT) & MDP5_PIPE_SW_PIX_EXT_REQ_PIXELS_TOP_BOTTOM__MASK;
+}
+
 static inline uint32_t REG_MDP5_PIPE_SCALE_CONFIG(enum mdp5_pipe i0) { return 0x00000204 + __offset_PIPE(i0); }
 #define MDP5_PIPE_SCALE_CONFIG_SCALEX_EN			0x00000001
 #define MDP5_PIPE_SCALE_CONFIG_SCALEY_EN			0x00000002
diff --git a/drivers/gpu/drm/msm/mdp/mdp_common.xml.h b/drivers/gpu/drm/msm/mdp/mdp_common.xml.h
index 4f792c4..2916c4d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp_common.xml.h
+++ b/drivers/gpu/drm/msm/mdp/mdp_common.xml.h
@@ -11,8 +11,8 @@ The rules-ng-ng source files this header was generated from are:
 - /home/robclark/src/freedreno/envytools/rnndb/msm.xml                 (    676 bytes, from 2015-05-20 20:03:14)
 - /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml (   1453 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp4.xml            (  20915 bytes, from 2015-05-20 20:03:14)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2576 bytes, from 2015-07-09 22:10:24)
-- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  36021 bytes, from 2015-07-09 22:10:24)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp_common.xml      (   2849 bytes, from 2015-09-18 12:07:28)
+- /home/robclark/src/freedreno/envytools/rnndb/mdp/mdp5.xml            (  37194 bytes, from 2015-09-18 12:07:28)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/dsi.xml             (  26057 bytes, from 2015-08-14 21:47:57)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/sfpb.xml            (    344 bytes, from 2015-05-20 20:03:07)
 - /home/robclark/src/freedreno/envytools/rnndb/dsi/mmss_cc.xml         (   1686 bytes, from 2015-05-20 20:03:14)
@@ -78,6 +78,13 @@ enum mdp_alpha_type {
 	BG_PIXEL = 3,
 };
 
+enum mdp_component_type {
+	COMP_0 = 0,
+	COMP_1_2 = 1,
+	COMP_3 = 2,
+	COMP_MAX = 3,
+};
+
 enum mdp_bpc {
 	BPC1 = 0,
 	BPC5 = 1,
diff --git a/drivers/gpu/drm/msm/ocmem/ocmem.xml.h b/drivers/gpu/drm/msm/ocmem/ocmem.xml.h
new file mode 100644
index 0000000..64ec02e
--- /dev/null
+++ b/drivers/gpu/drm/msm/ocmem/ocmem.xml.h
@@ -0,0 +1,113 @@
+#ifndef OCMEM_XML
+#define OCMEM_XML
+
+/* Autogenerated file, DO NOT EDIT manually!
+
+This file was generated by the rules-ng-ng headergen tool in this git repository:
+http://github.com/freedreno/envytools/
+git clone https://github.com/freedreno/envytools.git
+
+The rules-ng-ng source files this header was generated from are:
+- /home/robclark/src/freedreno/envytools/rnndb/adreno.xml               (    398 bytes, from 2015-09-24 17:25:31)
+- /home/robclark/src/freedreno/envytools/rnndb/freedreno_copyright.xml  (   1453 bytes, from 2015-05-20 20:03:07)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a2xx.xml          (  32901 bytes, from 2015-05-20 20:03:14)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_common.xml (  10755 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/adreno_pm4.xml    (  14968 bytes, from 2015-05-20 20:12:27)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a3xx.xml          (  67771 bytes, from 2015-09-14 20:46:55)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/a4xx.xml          (  63970 bytes, from 2015-09-14 20:50:12)
+- /home/robclark/src/freedreno/envytools/rnndb/adreno/ocmem.xml         (   1773 bytes, from 2015-09-24 17:30:00)
+
+Copyright (C) 2013-2015 by the following authors:
+- Rob Clark <robdclark@gmail.com> (robclark)
+
+Permission is hereby granted, free of charge, to any person obtaining
+a copy of this software and associated documentation files (the
+"Software"), to deal in the Software without restriction, including
+without limitation the rights to use, copy, modify, merge, publish,
+distribute, sublicense, and/or sell copies of the Software, and to
+permit persons to whom the Software is furnished to do so, subject to
+the following conditions:
+
+The above copyright notice and this permission notice (including the
+next paragraph) shall be included in all copies or substantial
+portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+IN NO EVENT SHALL THE COPYRIGHT OWNER(S) AND/OR ITS SUPPLIERS BE
+LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+
+enum ocmem_macro_state {
+	PASSTHROUGH = 0,
+	PERI_ON = 1,
+	CORE_ON = 2,
+	CLK_OFF = 4,
+};
+
+#define REG_OCMEM_HW_VERSION					0x00000000
+
+#define REG_OCMEM_HW_PROFILE					0x00000004
+#define OCMEM_HW_PROFILE_NUM_PORTS__MASK			0x0000000f
+#define OCMEM_HW_PROFILE_NUM_PORTS__SHIFT			0
+static inline uint32_t OCMEM_HW_PROFILE_NUM_PORTS(uint32_t val)
+{
+	return ((val) << OCMEM_HW_PROFILE_NUM_PORTS__SHIFT) & OCMEM_HW_PROFILE_NUM_PORTS__MASK;
+}
+#define OCMEM_HW_PROFILE_NUM_MACROS__MASK			0x00003f00
+#define OCMEM_HW_PROFILE_NUM_MACROS__SHIFT			8
+static inline uint32_t OCMEM_HW_PROFILE_NUM_MACROS(uint32_t val)
+{
+	return ((val) << OCMEM_HW_PROFILE_NUM_MACROS__SHIFT) & OCMEM_HW_PROFILE_NUM_MACROS__MASK;
+}
+#define OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE			0x00010000
+#define OCMEM_HW_PROFILE_INTERLEAVING				0x00020000
+
+#define REG_OCMEM_GEN_STATUS					0x0000000c
+
+#define REG_OCMEM_PSGSC_STATUS					0x00000038
+
+static inline uint32_t REG_OCMEM_PSGSC(uint32_t i0) { return 0x0000003c + 0x1*i0; }
+
+static inline uint32_t REG_OCMEM_PSGSC_CTL(uint32_t i0) { return 0x0000003c + 0x1*i0; }
+#define OCMEM_PSGSC_CTL_MACRO0_MODE__MASK			0x00000007
+#define OCMEM_PSGSC_CTL_MACRO0_MODE__SHIFT			0
+static inline uint32_t OCMEM_PSGSC_CTL_MACRO0_MODE(enum ocmem_macro_state val)
+{
+	return ((val) << OCMEM_PSGSC_CTL_MACRO0_MODE__SHIFT) & OCMEM_PSGSC_CTL_MACRO0_MODE__MASK;
+}
+#define OCMEM_PSGSC_CTL_MACRO1_MODE__MASK			0x00000070
+#define OCMEM_PSGSC_CTL_MACRO1_MODE__SHIFT			4
+static inline uint32_t OCMEM_PSGSC_CTL_MACRO1_MODE(enum ocmem_macro_state val)
+{
+	return ((val) << OCMEM_PSGSC_CTL_MACRO1_MODE__SHIFT) & OCMEM_PSGSC_CTL_MACRO1_MODE__MASK;
+}
+#define OCMEM_PSGSC_CTL_MACRO2_MODE__MASK			0x00000700
+#define OCMEM_PSGSC_CTL_MACRO2_MODE__SHIFT			8
+static inline uint32_t OCMEM_PSGSC_CTL_MACRO2_MODE(enum ocmem_macro_state val)
+{
+	return ((val) << OCMEM_PSGSC_CTL_MACRO2_MODE__SHIFT) & OCMEM_PSGSC_CTL_MACRO2_MODE__MASK;
+}
+#define OCMEM_PSGSC_CTL_MACRO3_MODE__MASK			0x00007000
+#define OCMEM_PSGSC_CTL_MACRO3_MODE__SHIFT			12
+static inline uint32_t OCMEM_PSGSC_CTL_MACRO3_MODE(enum ocmem_macro_state val)
+{
+	return ((val) << OCMEM_PSGSC_CTL_MACRO3_MODE__SHIFT) & OCMEM_PSGSC_CTL_MACRO3_MODE__MASK;
+}
+
+#define REG_OCMEM_REGION_MODE_CTL				0x00001000
+#define OCMEM_REGION_MODE_CTL_REG0_THIN				0x00000001
+#define OCMEM_REGION_MODE_CTL_REG1_THIN				0x00000002
+#define OCMEM_REGION_MODE_CTL_REG2_THIN				0x00000004
+#define OCMEM_REGION_MODE_CTL_REG3_THIN				0x00000008
+
+#define REG_OCMEM_GFX_MPU_START					0x00001004
+
+#define REG_OCMEM_GFX_MPU_END					0x00001008
+
+
+#endif /* OCMEM_XML */
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] drm/msm: add OCMEM driver
  2015-09-28 18:51 [PATCH 0/4] Add OCMEM support Rob Clark
                   ` (2 preceding siblings ...)
  2015-09-28 18:51 ` [PATCH 3/4] drm/msm: update generated headers Rob Clark
@ 2015-09-28 18:51 ` Rob Clark
  2015-09-28 22:10   ` Stephen Boyd
  3 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2015-09-28 18:51 UTC (permalink / raw)
  To: dri-devel, linux-arm-msm; +Cc: Stephen Boyd, Bjorn Andersson, Rob Clark

For now, since the GPU is the only upstream consumer, just stuff this
into drm/msm.  Eventually if we have other consumers, we'll have to
split this out and make the allocation less hard coded.  But I'll punt
on that until I better understand the non-gpu uses-cases (and whether
the allocation *really* needs to be as complicated as it is in the
downstream driver).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/Makefile          |   3 +-
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c |  17 +-
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c |  19 +-
 drivers/gpu/drm/msm/msm_drv.c         |   2 +
 drivers/gpu/drm/msm/msm_gpu.h         |   3 +
 drivers/gpu/drm/msm/ocmem/ocmem.c     | 396 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/ocmem/ocmem.h     |  46 ++++
 7 files changed, 459 insertions(+), 27 deletions(-)
 create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.c
 create mode 100644 drivers/gpu/drm/msm/ocmem/ocmem.h

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 0a543eb..8ddf6fa 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -48,7 +48,8 @@ msm-y := \
 	msm_iommu.o \
 	msm_perf.o \
 	msm_rd.o \
-	msm_ringbuffer.o
+	msm_ringbuffer.o \
+	ocmem/ocmem.o
 
 msm-$(CONFIG_DRM_MSM_FBDEV) += msm_fbdev.o
 msm-$(CONFIG_COMMON_CLK) += mdp/mdp4/mdp4_lvds_pll.o
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index ca29688..29bbb80 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -17,10 +17,7 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifdef CONFIG_MSM_OCMEM
-#  include <mach/ocmem.h>
-#endif
-
+#include "ocmem/ocmem.h"
 #include "a3xx_gpu.h"
 
 #define A3XX_INT0_MASK \
@@ -322,10 +319,8 @@ static void a3xx_destroy(struct msm_gpu *gpu)
 
 	adreno_gpu_cleanup(adreno_gpu);
 
-#ifdef CONFIG_MSM_OCMEM
 	if (a3xx_gpu->ocmem_base)
 		ocmem_free(OCMEM_GRAPHICS, a3xx_gpu->ocmem_hdl);
-#endif
 
 	kfree(a3xx_gpu);
 }
@@ -539,6 +534,7 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev)
 	struct msm_gpu *gpu;
 	struct msm_drm_private *priv = dev->dev_private;
 	struct platform_device *pdev = priv->gpu_pdev;
+	struct ocmem_buf *ocmem_hdl;
 	int ret;
 
 	if (!pdev) {
@@ -569,18 +565,13 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev)
 		goto fail;
 
 	/* if needed, allocate gmem: */
-	if (adreno_is_a330(adreno_gpu)) {
-#ifdef CONFIG_MSM_OCMEM
-		/* TODO this is different/missing upstream: */
-		struct ocmem_buf *ocmem_hdl =
-				ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem);
-
+	ocmem_hdl = ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem);
+	if (!IS_ERR(ocmem_hdl)) {
 		a3xx_gpu->ocmem_hdl = ocmem_hdl;
 		a3xx_gpu->ocmem_base = ocmem_hdl->addr;
 		adreno_gpu->gmem = ocmem_hdl->len;
 		DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024,
 				a3xx_gpu->ocmem_base);
-#endif
 	}
 
 	if (!gpu->mmu) {
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index a53f1be..17f084d 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -10,10 +10,9 @@
  * GNU General Public License for more details.
  *
  */
+
+#include "ocmem/ocmem.h"
 #include "a4xx_gpu.h"
-#ifdef CONFIG_MSM_OCMEM
-#  include <soc/qcom/ocmem.h>
-#endif
 
 #define A4XX_INT0_MASK \
 	(A4XX_INT0_RBBM_AHB_ERROR |        \
@@ -289,10 +288,8 @@ static void a4xx_destroy(struct msm_gpu *gpu)
 
 	adreno_gpu_cleanup(adreno_gpu);
 
-#ifdef CONFIG_MSM_OCMEM
-	if (a4xx_gpu->ocmem_base)
+	if (a4xx_gpu->ocmem_hdl)
 		ocmem_free(OCMEM_GRAPHICS, a4xx_gpu->ocmem_hdl);
-#endif
 
 	kfree(a4xx_gpu);
 }
@@ -538,6 +535,7 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev)
 	struct msm_gpu *gpu;
 	struct msm_drm_private *priv = dev->dev_private;
 	struct platform_device *pdev = priv->gpu_pdev;
+	struct ocmem_buf *ocmem_hdl;
 	int ret;
 
 	if (!pdev) {
@@ -568,18 +566,13 @@ struct msm_gpu *a4xx_gpu_init(struct drm_device *dev)
 		goto fail;
 
 	/* if needed, allocate gmem: */
-	if (adreno_is_a4xx(adreno_gpu)) {
-#ifdef CONFIG_MSM_OCMEM
-		/* TODO this is different/missing upstream: */
-		struct ocmem_buf *ocmem_hdl =
-				ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem);
-
+	ocmem_hdl = ocmem_allocate(OCMEM_GRAPHICS, adreno_gpu->gmem);
+	if (!IS_ERR(ocmem_hdl)) {
 		a4xx_gpu->ocmem_hdl = ocmem_hdl;
 		a4xx_gpu->ocmem_base = ocmem_hdl->addr;
 		adreno_gpu->gmem = ocmem_hdl->len;
 		DBG("using %dK of OCMEM at 0x%08x", adreno_gpu->gmem / 1024,
 				a4xx_gpu->ocmem_base);
-#endif
 	}
 
 	if (!gpu->mmu) {
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 28c9a2a..1b02c2d 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1165,6 +1165,7 @@ static int __init msm_drm_register(void)
 	msm_dsi_register();
 	msm_edp_register();
 	hdmi_register();
+	ocmem_register();
 	adreno_register();
 	return platform_driver_register(&msm_platform_driver);
 }
@@ -1175,6 +1176,7 @@ static void __exit msm_drm_unregister(void)
 	platform_driver_unregister(&msm_platform_driver);
 	hdmi_unregister();
 	adreno_unregister();
+	ocmem_unregister();
 	msm_edp_unregister();
 	msm_dsi_unregister();
 }
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 2bbe85a..f042ba8 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -172,4 +172,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev);
 void __init adreno_register(void);
 void __exit adreno_unregister(void);
 
+void __init ocmem_register(void);
+void __exit ocmem_unregister(void);
+
 #endif /* __MSM_GPU_H__ */
diff --git a/drivers/gpu/drm/msm/ocmem/ocmem.c b/drivers/gpu/drm/msm/ocmem/ocmem.c
new file mode 100644
index 0000000..d3cdd64
--- /dev/null
+++ b/drivers/gpu/drm/msm/ocmem/ocmem.c
@@ -0,0 +1,396 @@
+/*
+ * Copyright (C) 2015 Red Hat
+ * Author: Rob Clark <robdclark@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/cpuset.h>
+#include <linux/qcom_scm.h>
+
+#include "msm_drv.h"
+#include "ocmem.h"
+#include "ocmem.xml.h"
+
+enum region_mode {
+	WIDE_MODE = 0x0,
+	THIN_MODE,
+	MODE_DEFAULT = WIDE_MODE,
+};
+
+enum ocmem_tz_client {
+	TZ_UNUSED = 0x0,
+	TZ_GRAPHICS,
+	TZ_VIDEO,
+	TZ_LP_AUDIO,
+	TZ_SENSORS,
+	TZ_OTHER_OS,
+	TZ_DEBUG,
+};
+
+struct ocmem_region {
+	unsigned psgsc_ctrl;
+	bool interleaved;
+	enum region_mode mode;
+	unsigned int num_macros;
+	enum ocmem_macro_state macro_state[4];
+	unsigned long macro_size;
+	unsigned long region_size;
+};
+
+struct ocmem_config {
+	uint8_t  num_regions;
+	uint32_t macro_size;
+};
+
+struct ocmem {
+	struct device *dev;
+	const struct ocmem_config *config;
+	struct resource *ocmem_mem;
+	struct clk *core_clk;
+	struct clk *iface_clk;
+	void __iomem *mmio;
+
+	unsigned num_ports;
+	unsigned num_macros;
+	bool interleaved;
+
+	struct ocmem_region *regions;
+};
+
+struct ocmem *ocmem;
+
+static bool ocmem_exists(void);
+
+static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
+{
+	msm_writel(data, ocmem->mmio + reg);
+}
+
+static inline u32 ocmem_read(struct ocmem *ocmem, u32 reg)
+{
+	return msm_readl(ocmem->mmio + reg);
+}
+
+static int ocmem_clk_enable(struct ocmem *ocmem)
+{
+	int ret;
+
+	ret = clk_prepare_enable(ocmem->core_clk);
+	if (ret)
+		return ret;
+
+	if (ocmem->iface_clk) {
+		ret = clk_prepare_enable(ocmem->iface_clk);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void ocmem_clk_disable(struct ocmem *ocmem)
+{
+	if (ocmem->iface_clk)
+		clk_disable_unprepare(ocmem->iface_clk);
+	clk_disable_unprepare(ocmem->core_clk);
+}
+
+static int ocmem_dev_remove(struct platform_device *pdev)
+{
+	ocmem_clk_disable(ocmem);
+	return 0;
+}
+
+static void update_ocmem(struct ocmem *ocmem)
+{
+	uint32_t region_mode_ctrl = 0x0;
+	unsigned pos = 0;
+	unsigned i = 0;
+
+	if (!qcom_scm_ocmem_lock_available()) {
+		for (i = 0; i < ocmem->config->num_regions; i++) {
+			struct ocmem_region *region = &ocmem->regions[i];
+			pos = i << 2;
+			if (region->mode == THIN_MODE)
+				region_mode_ctrl |= BIT(pos);
+		}
+		dev_dbg(ocmem->dev, "ocmem_region_mode_control %x\n", region_mode_ctrl);
+		ocmem_write(ocmem, REG_OCMEM_REGION_MODE_CTL, region_mode_ctrl);
+		/* Barrier to commit the region mode */
+		mb();
+	}
+
+	for (i = 0; i < ocmem->config->num_regions; i++) {
+		struct ocmem_region *region = &ocmem->regions[i];
+
+		ocmem_write(ocmem, REG_OCMEM_PSGSC_CTL(i),
+				OCMEM_PSGSC_CTL_MACRO0_MODE(region->macro_state[0]) |
+				OCMEM_PSGSC_CTL_MACRO1_MODE(region->macro_state[1]) |
+				OCMEM_PSGSC_CTL_MACRO2_MODE(region->macro_state[2]) |
+				OCMEM_PSGSC_CTL_MACRO3_MODE(region->macro_state[3]));
+	}
+}
+
+inline unsigned long phys_to_offset(unsigned long addr)
+{
+	if ((addr < ocmem->ocmem_mem->start) ||
+		(addr >= ocmem->ocmem_mem->end))
+		return 0;
+	return addr - ocmem->ocmem_mem->start;
+}
+
+static unsigned long device_address(enum ocmem_client client, unsigned long addr)
+{
+	/* TODO, gpu uses phys_to_offset, but others do not.. */
+	return phys_to_offset(addr);
+}
+
+static void update_range(struct ocmem *ocmem, struct ocmem_buf *buf,
+		enum ocmem_macro_state mstate, enum region_mode rmode)
+{
+	unsigned long offset = 0;
+	int i, j;
+
+	/*
+	 * TODO probably should assert somewhere that range is aligned
+	 * to macro boundaries..
+	 */
+
+	for (i = 0; i < ocmem->config->num_regions; i++) {
+		struct ocmem_region *region = &ocmem->regions[i];
+		if ((buf->offset <= offset) && (offset < (buf->offset + buf->len)))
+			region->mode = rmode;
+		for (j = 0; j < region->num_macros; j++) {
+			if ((buf->offset <= offset) && (offset < (buf->offset + buf->len)))
+				region->macro_state[j] = mstate;
+			offset += region->macro_size;
+		}
+	}
+
+	update_ocmem(ocmem);
+}
+
+struct ocmem_buf *ocmem_allocate(enum ocmem_client client, unsigned long size)
+{
+	struct ocmem_buf *buf;
+
+	if (!ocmem) {
+		if (ocmem_exists())
+			return ERR_PTR(-EPROBE_DEFER);
+		return ERR_PTR(-ENXIO);
+	}
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+
+	/*
+	 * TODO less hard-coded allocation that works for more than
+	 * one user:
+	 */
+
+	buf->offset = 0;
+	buf->addr = device_address(client, buf->offset);
+	buf->len = size;
+
+	update_range(ocmem, buf, CORE_ON, WIDE_MODE);
+
+	if (qcom_scm_ocmem_lock_available()) {
+		int ret;
+		ret = qcom_scm_ocmem_lock(TZ_GRAPHICS, buf->offset, buf->len,
+				WIDE_MODE);
+		if (ret)
+			dev_err(ocmem->dev, "could not lock: %d\n", ret);
+	} else {
+		if (client == OCMEM_GRAPHICS) {
+			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_START, buf->offset);
+			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_END, buf->offset + buf->len);
+		}
+	}
+
+	return buf;
+}
+
+void ocmem_free(enum ocmem_client client, struct ocmem_buf *buf)
+{
+	update_range(ocmem, buf, CLK_OFF, MODE_DEFAULT);
+
+	if (qcom_scm_ocmem_lock_available()) {
+		int ret;
+		ret = qcom_scm_ocmem_unlock(TZ_GRAPHICS, buf->offset, buf->len);
+		if (ret)
+			dev_err(ocmem->dev, "could not lock: %d\n", ret);
+	} else {
+		if (client == OCMEM_GRAPHICS) {
+			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_START, 0x0);
+			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_END, 0x0);
+		}
+	}
+
+	kfree(buf);
+}
+
+static const struct ocmem_config ocmem_8974_config = {
+	.num_regions = 3, .macro_size = SZ_128K,
+};
+
+static const struct of_device_id dt_match[] = {
+	{ .compatible = "qcom,msm-ocmem-8974", .data = &ocmem_8974_config },
+	{}
+};
+
+static int ocmem_dev_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct ocmem_config *config = NULL;
+	uint32_t reg, num_banks, region_size;
+	int i, j, ret;
+
+	struct device_node *of_node = dev->of_node;
+	const struct of_device_id *match;
+
+	match = of_match_node(dt_match, dev->of_node);
+	if (match)
+		config = match->data;
+
+	if (!config) {
+		dev_err(dev, "unknown config: %s\n", of_node->name);
+		return -ENXIO;
+	}
+
+	ocmem = devm_kzalloc(dev, sizeof(*ocmem), GFP_KERNEL);
+	if (!ocmem)
+		return -ENOMEM;
+
+	ocmem->dev = dev;
+	ocmem->config = config;
+
+	ocmem->core_clk = devm_clk_get(dev, "core_clk");
+	if (IS_ERR(ocmem->core_clk)) {
+		dev_err(dev, "Unable to get the core clock\n");
+		return PTR_ERR(ocmem->core_clk);
+	}
+
+	ocmem->iface_clk = devm_clk_get(dev, "iface_clk");
+	if (IS_ERR_OR_NULL(ocmem->iface_clk))
+		ocmem->iface_clk = NULL;
+
+	/* The core clock is synchronous with graphics */
+	WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
+
+	ocmem->mmio = msm_ioremap(pdev, "ocmem_ctrl_physical", "OCMEM");
+	if (IS_ERR(ocmem->mmio))
+		return PTR_ERR(ocmem->mmio);
+
+	ocmem->ocmem_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+			"ocmem_physical");
+	if (!ocmem->ocmem_mem) {
+		dev_err(dev, "could not get OCMEM region\n");
+		return -ENXIO;
+	}
+
+	ret = ocmem_clk_enable(ocmem);
+	if (ret)
+		goto fail;
+
+	if (qcom_scm_is_available() && qcom_scm_ocmem_secure_available()) {
+		dev_info(dev, "configuring scm\n");
+		ret = qcom_scm_ocmem_secure_cfg(0x5);
+		if (ret)
+			goto fail;
+	}
+
+	reg = ocmem_read(ocmem, REG_OCMEM_HW_PROFILE);
+	ocmem->num_ports = FIELD(reg, OCMEM_HW_PROFILE_NUM_PORTS);
+	ocmem->num_macros = FIELD(reg, OCMEM_HW_PROFILE_NUM_MACROS);
+	ocmem->interleaved = !!(reg & OCMEM_HW_PROFILE_INTERLEAVING);
+
+	num_banks = ocmem->num_ports / 2;
+	region_size = config->macro_size * num_banks;
+
+	dev_info(dev, "%u ports, %u regions, %u macros, %sinterleaved\n",
+			ocmem->num_ports, config->num_regions, ocmem->num_macros,
+			ocmem->interleaved ? "" : "not ");
+
+	ocmem->regions = devm_kzalloc(dev, sizeof(struct ocmem_region) *
+			config->num_regions, GFP_KERNEL);
+	if (!ocmem->regions) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	for (i = 0; i < config->num_regions; i++) {
+		struct ocmem_region *region = &ocmem->regions[i];
+
+		if (WARN_ON(num_banks > ARRAY_SIZE(region->macro_state))) {
+			ret = -EINVAL;
+			goto fail;
+		}
+
+		region->mode = MODE_DEFAULT;
+		region->num_macros = num_banks;
+
+		if ((i == (config->num_regions - 1)) &&
+				(reg & OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE)) {
+			region->macro_size = config->macro_size / 2;
+			region->region_size = region_size / 2;
+		} else {
+			region->macro_size = config->macro_size;
+			region->region_size = region_size;
+		}
+
+		for (j = 0; j < ARRAY_SIZE(region->macro_state); j++)
+			region->macro_state[j] = CLK_OFF;
+	}
+
+	return 0;
+
+fail:
+	dev_err(dev, "probe failed\n");
+	ocmem_dev_remove(pdev);
+	return ret;
+}
+
+static struct platform_driver ocmem_driver = {
+	.probe = ocmem_dev_probe,
+	.remove = ocmem_dev_remove,
+	.driver = {
+		.name = "ocmem",
+		.of_match_table = dt_match,
+	},
+};
+
+static bool ocmem_exists(void)
+{
+	struct device_driver *drv = &ocmem_driver.driver;
+	struct device *d;
+
+	d = bus_find_device(&platform_bus_type, NULL, drv,
+			(void *)platform_bus_type.match);
+	if (d) {
+		put_device(d);
+		return true;
+	}
+
+	return false;
+}
+
+void __init ocmem_register(void)
+{
+	platform_driver_register(&ocmem_driver);
+}
+
+void __exit ocmem_unregister(void)
+{
+	platform_driver_unregister(&ocmem_driver);
+}
diff --git a/drivers/gpu/drm/msm/ocmem/ocmem.h b/drivers/gpu/drm/msm/ocmem/ocmem.h
new file mode 100644
index 0000000..199be98
--- /dev/null
+++ b/drivers/gpu/drm/msm/ocmem/ocmem.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2015 Red Hat
+ * Author: Rob Clark <robdclark@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __OCMEM_H__
+#define __OCMEM_H__
+
+enum ocmem_client {
+	/* GMEM clients */
+	OCMEM_GRAPHICS = 0x0,
+	/* TCMEM clients */
+	OCMEM_VIDEO,
+	OCMEM_CAMERA,
+	/* Dummy Clients */
+	OCMEM_HP_AUDIO,
+	OCMEM_VOICE,
+	/* IMEM Clients */
+	OCMEM_LP_AUDIO,
+	OCMEM_SENSORS,
+	OCMEM_OTHER_OS,
+	OCMEM_CLIENT_MAX,
+};
+
+struct ocmem_buf {
+	unsigned long offset;
+	unsigned long addr;
+	unsigned long len;
+};
+
+struct ocmem_buf *ocmem_allocate(enum ocmem_client client, unsigned long size);
+void ocmem_free(enum ocmem_client client, struct ocmem_buf *buf);
+
+#endif /* __OCMEM_H__ */
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] qcom-scm: add ocmem support
  2015-09-28 18:51 ` [PATCH 1/4] qcom-scm: add ocmem support Rob Clark
@ 2015-09-28 20:51   ` Stephen Boyd
  2015-09-28 21:08     ` Rob Clark
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Boyd @ 2015-09-28 20:51 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-arm-msm, Bjorn Andersson

On 09/28, Rob Clark wrote:
> Add interfaces needed for configuring OCMEM.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/firmware/qcom_scm-32.c |  57 ++++++++++++++++++++++
>  drivers/firmware/qcom_scm-64.c |  16 +++++++
>  drivers/firmware/qcom_scm.c    | 106 +++++++++++++++++++++++++++++++++++++++++
>  drivers/firmware/qcom_scm.h    |  13 +++++
>  include/linux/qcom_scm.h       |  10 ++++
>  5 files changed, 202 insertions(+)
> 
> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
> index e9c306b..656d8fe 100644
> --- a/drivers/firmware/qcom_scm-32.c
> +++ b/drivers/firmware/qcom_scm-32.c
> @@ -500,6 +500,63 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>  		req, req_cnt * sizeof(*req), resp, sizeof(*resp));
>  }
>  
> +int __qcom_scm_ocmem_secure_cfg(unsigned sec_id)
> +{
> +	int ret, scm_ret = 0;
> +	struct msm_scm_sec_cfg {
> +		unsigned int id;
> +		unsigned int spare;


__le32 for both

> +	} cfg;
> +
> +	cfg.id = sec_id;
> +
> +

nitpick: drop double space

> +	ret = qcom_scm_call(QCOM_SCM_OCMEM_SECURE_SVC, QCOM_SCM_OCMEM_SECURE_CFG,
> +			&cfg, sizeof(cfg), &scm_ret, sizeof(scm_ret));
> +
> +	if (ret || scm_ret) {
> +		pr_err("ocmem: Failed to enable secure programming\n");

Maybe the caller should print something if they care instead of
burying it down here.

> +		return ret ? ret : -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +int __qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
> +		uint32_t mode)

Please use u32 here instead of uint32_t. uint32_t is not for
kernel code.

> +{
> +	struct ocmem_tz_lock {
> +		u32 id;
> +		u32 offset;
> +		u32 size;
> +		u32 mode;

All __le32

> +	} request;
> +
> +	request.id = id;
> +	request.offset = offset;
> +	request.size = size;
> +	request.mode = mode;

And then do the cpu_to_le32() stuff here.

> +
> +	return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_LOCK_CMD,
> +			&request, sizeof(request), NULL, 0);
> +}
> +
> +int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)

u32

> +{
> +	struct ocmem_tz_unlock {
> +		u32 id;
> +		u32 offset;
> +		u32 size;

__le32

> +	} request;
> +
> +	request.id = id;
> +	request.offset = offset;
> +	request.size = size;
> +
> +	return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_UNLOCK_CMD,
> +			&request, sizeof(request), NULL, 0);
> +}
> +
>  bool __qcom_scm_pas_supported(u32 peripheral)
>  {
>  	__le32 out;
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index 118df0a..59b1007 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -154,6 +154,112 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>  EXPORT_SYMBOL(qcom_scm_hdcp_req);
>  
>  /**
> + * qcom_scm_ocmem_secure_available() - Check if secure environment supports
> + * OCMEM.
> + *
> + * Return true if OCMEM secure interface is supported, false if not.
> + */
> +bool qcom_scm_ocmem_secure_available(void)
> +{
> +	int ret = qcom_scm_clk_enable();

I doubt we need to enable clocks to figure out if a call is
available. Please drop clk stuff here.

> +
> +	if (ret)
> +		goto clk_err;
> +
> +	ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SECURE_SVC,
> +			QCOM_SCM_OCMEM_SECURE_CFG);
> +
> +	qcom_scm_clk_disable();
> +
> +clk_err:
> +	return (ret > 0) ? true : false;
> +}
> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_available);
> +
> +/**
> + * qcom_scm_ocmem_secure_cfg() - call OCMEM secure cfg interface
> + */
> +int qcom_scm_ocmem_secure_cfg(unsigned sec_id)
> +{
> +	int ret = qcom_scm_clk_enable();
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = __qcom_scm_ocmem_secure_cfg(sec_id);
> +	qcom_scm_clk_disable();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_cfg);
> +
> +/**
> + * qcom_scm_ocmem_lock_available() - is OCMEM lock/unlock interface available
> + */
> +bool qcom_scm_ocmem_lock_available(void)
> +{
> +	int ret = qcom_scm_clk_enable();

No need for clocks?

> +
> +	if (ret)
> +		goto clk_err;
> +
> +	ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SVC,
> +			QCOM_SCM_OCMEM_LOCK_CMD);
> +
> +	qcom_scm_clk_disable();
> +
> +clk_err:
> +	return (ret > 0) ? true : false;
> +}
> +EXPORT_SYMBOL(qcom_scm_ocmem_lock_available);
> +
> +/**
> + * qcom_scm_ocmem_lock() - call OCMEM lock interface to assign an OCMEM
> + * region to the specified initiator
> + *
> + * @id:     tz initiator id
> + * @offset: OCMEM offset
> + * @size:   OCMEM size
> + * @mode:   access mode (WIDE/NARROW)
> + */
> +int qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
> +		uint32_t mode)
> +{
> +	int ret = qcom_scm_clk_enable();
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = __qcom_scm_ocmem_lock(id, offset, size, mode);
> +	qcom_scm_clk_disable();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_ocmem_lock);
> +
> +/**
> + * qcom_scm_ocmem_unlock() - call OCMEM unlock interface to release an OCMEM
> + * region from the specified initiator
> + *
> + * @id:     tz initiator id
> + * @offset: OCMEM offset
> + * @size:   OCMEM size
> + */
> +int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
> +{
> +	int ret = qcom_scm_clk_enable();
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = __qcom_scm_ocmem_unlock(id, offset, size);
> +	qcom_scm_clk_disable();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(qcom_scm_ocmem_unlock);

I don't think we need any clocks for lock/unlock/cfg either. The
scm clocks are some crypto clocks that the secure side isn't able
to enable and we don't have a device in DT for them. In the ocmem
case, we should rely on the ocmem device to get the clocks and
turn them on before calling any scm APIs that may require those
clocks.

> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 46d41e4..a934457 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -23,10 +23,20 @@ struct qcom_scm_hdcp_req {
>  	u32 val;
>  };
>  
> +extern bool qcom_scm_is_available(void);

Is this used? Looks like noise.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] qcom-scm: add ocmem support
  2015-09-28 20:51   ` Stephen Boyd
@ 2015-09-28 21:08     ` Rob Clark
  2015-09-28 21:59       ` Bjorn Andersson
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2015-09-28 21:08 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: dri-devel, linux-arm-msm, Bjorn Andersson

On Mon, Sep 28, 2015 at 4:51 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 09/28, Rob Clark wrote:
>> Add interfaces needed for configuring OCMEM.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/firmware/qcom_scm-32.c |  57 ++++++++++++++++++++++
>>  drivers/firmware/qcom_scm-64.c |  16 +++++++
>>  drivers/firmware/qcom_scm.c    | 106 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/firmware/qcom_scm.h    |  13 +++++
>>  include/linux/qcom_scm.h       |  10 ++++
>>  5 files changed, 202 insertions(+)
>>
>> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
>> index e9c306b..656d8fe 100644
>> --- a/drivers/firmware/qcom_scm-32.c
>> +++ b/drivers/firmware/qcom_scm-32.c
>> @@ -500,6 +500,63 @@ int __qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>>               req, req_cnt * sizeof(*req), resp, sizeof(*resp));
>>  }
>>
>> +int __qcom_scm_ocmem_secure_cfg(unsigned sec_id)
>> +{
>> +     int ret, scm_ret = 0;
>> +     struct msm_scm_sec_cfg {
>> +             unsigned int id;
>> +             unsigned int spare;
>
>
> __le32 for both
>
>> +     } cfg;
>> +
>> +     cfg.id = sec_id;
>> +
>> +
>
> nitpick: drop double space
>
>> +     ret = qcom_scm_call(QCOM_SCM_OCMEM_SECURE_SVC, QCOM_SCM_OCMEM_SECURE_CFG,
>> +                     &cfg, sizeof(cfg), &scm_ret, sizeof(scm_ret));
>> +
>> +     if (ret || scm_ret) {
>> +             pr_err("ocmem: Failed to enable secure programming\n");
>
> Maybe the caller should print something if they care instead of
> burying it down here.
>
>> +             return ret ? ret : -EINVAL;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +int __qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
>> +             uint32_t mode)
>
> Please use u32 here instead of uint32_t. uint32_t is not for
> kernel code.
>
>> +{
>> +     struct ocmem_tz_lock {
>> +             u32 id;
>> +             u32 offset;
>> +             u32 size;
>> +             u32 mode;
>
> All __le32
>
>> +     } request;
>> +
>> +     request.id = id;
>> +     request.offset = offset;
>> +     request.size = size;
>> +     request.mode = mode;
>
> And then do the cpu_to_le32() stuff here.
>
>> +
>> +     return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_LOCK_CMD,
>> +                     &request, sizeof(request), NULL, 0);
>> +}
>> +
>> +int __qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
>
> u32
>
>> +{
>> +     struct ocmem_tz_unlock {
>> +             u32 id;
>> +             u32 offset;
>> +             u32 size;
>
> __le32
>
>> +     } request;
>> +
>> +     request.id = id;
>> +     request.offset = offset;
>> +     request.size = size;
>> +
>> +     return qcom_scm_call(QCOM_SCM_OCMEM_SVC, QCOM_SCM_OCMEM_UNLOCK_CMD,
>> +                     &request, sizeof(request), NULL, 0);
>> +}
>> +
>>  bool __qcom_scm_pas_supported(u32 peripheral)
>>  {
>>       __le32 out;
>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>> index 118df0a..59b1007 100644
>> --- a/drivers/firmware/qcom_scm.c
>> +++ b/drivers/firmware/qcom_scm.c
>> @@ -154,6 +154,112 @@ int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt, u32 *resp)
>>  EXPORT_SYMBOL(qcom_scm_hdcp_req);
>>
>>  /**
>> + * qcom_scm_ocmem_secure_available() - Check if secure environment supports
>> + * OCMEM.
>> + *
>> + * Return true if OCMEM secure interface is supported, false if not.
>> + */
>> +bool qcom_scm_ocmem_secure_available(void)
>> +{
>> +     int ret = qcom_scm_clk_enable();
>
> I doubt we need to enable clocks to figure out if a call is
> available. Please drop clk stuff here.

hmm, hdcp did, but pas didn't..  otoh it looks like the call to
__qcom_scm_pas_supported() *should* be wrapped in clk enable/disable..

And __qcom_scm_is_call_available() does call qcom_scm_call().  Which
is, I assume, what needs the clk's..  so not entirely sure if *all*
the clk enable/disable stuff should be stripped out, or if missing clk
stuff should be added in qcom_scm_pas_supported()..


>> +
>> +     if (ret)
>> +             goto clk_err;
>> +
>> +     ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SECURE_SVC,
>> +                     QCOM_SCM_OCMEM_SECURE_CFG);
>> +
>> +     qcom_scm_clk_disable();
>> +
>> +clk_err:
>> +     return (ret > 0) ? true : false;
>> +}
>> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_available);
>> +
>> +/**
>> + * qcom_scm_ocmem_secure_cfg() - call OCMEM secure cfg interface
>> + */
>> +int qcom_scm_ocmem_secure_cfg(unsigned sec_id)
>> +{
>> +     int ret = qcom_scm_clk_enable();
>> +
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = __qcom_scm_ocmem_secure_cfg(sec_id);
>> +     qcom_scm_clk_disable();
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_cfg);
>> +
>> +/**
>> + * qcom_scm_ocmem_lock_available() - is OCMEM lock/unlock interface available
>> + */
>> +bool qcom_scm_ocmem_lock_available(void)
>> +{
>> +     int ret = qcom_scm_clk_enable();
>
> No need for clocks?
>
>> +
>> +     if (ret)
>> +             goto clk_err;
>> +
>> +     ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SVC,
>> +                     QCOM_SCM_OCMEM_LOCK_CMD);
>> +
>> +     qcom_scm_clk_disable();
>> +
>> +clk_err:
>> +     return (ret > 0) ? true : false;
>> +}
>> +EXPORT_SYMBOL(qcom_scm_ocmem_lock_available);
>> +
>> +/**
>> + * qcom_scm_ocmem_lock() - call OCMEM lock interface to assign an OCMEM
>> + * region to the specified initiator
>> + *
>> + * @id:     tz initiator id
>> + * @offset: OCMEM offset
>> + * @size:   OCMEM size
>> + * @mode:   access mode (WIDE/NARROW)
>> + */
>> +int qcom_scm_ocmem_lock(uint32_t id, uint32_t offset, uint32_t size,
>> +             uint32_t mode)
>> +{
>> +     int ret = qcom_scm_clk_enable();
>> +
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = __qcom_scm_ocmem_lock(id, offset, size, mode);
>> +     qcom_scm_clk_disable();
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(qcom_scm_ocmem_lock);
>> +
>> +/**
>> + * qcom_scm_ocmem_unlock() - call OCMEM unlock interface to release an OCMEM
>> + * region from the specified initiator
>> + *
>> + * @id:     tz initiator id
>> + * @offset: OCMEM offset
>> + * @size:   OCMEM size
>> + */
>> +int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
>> +{
>> +     int ret = qcom_scm_clk_enable();
>> +
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = __qcom_scm_ocmem_unlock(id, offset, size);
>> +     qcom_scm_clk_disable();
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
>
> I don't think we need any clocks for lock/unlock/cfg either. The
> scm clocks are some crypto clocks that the secure side isn't able
> to enable and we don't have a device in DT for them. In the ocmem
> case, we should rely on the ocmem device to get the clocks and
> turn them on before calling any scm APIs that may require those
> clocks.

Hmm, if that is true then we should probably drop the clks for hdcp
fxns too, and maybe add a comment somewhere since it isn't really
clear what the clks are for (and when it is unclear, folks will just
cargo-cult what the existing fxns are doing).  As-is it is hard to
tell what is required and what is luck..

>> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
>> index 46d41e4..a934457 100644
>> --- a/include/linux/qcom_scm.h
>> +++ b/include/linux/qcom_scm.h
>> @@ -23,10 +23,20 @@ struct qcom_scm_hdcp_req {
>>       u32 val;
>>  };
>>
>> +extern bool qcom_scm_is_available(void);
>
> Is this used? Looks like noise.

perhaps should be split out into a separate patch..  but I am using
this, and it seems like a good idea to avoid null ptr deref's of
__scm.  Probably some of the scm callers should call this first..
either that or we should make other scm entry points behave better if
__scm is null..

BR,
-R

> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] qcom-scm: add ocmem support
  2015-09-28 21:08     ` Rob Clark
@ 2015-09-28 21:59       ` Bjorn Andersson
  2015-09-28 22:35         ` Stephen Boyd
  0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Andersson @ 2015-09-28 21:59 UTC (permalink / raw)
  To: Rob Clark; +Cc: Stephen Boyd, dri-devel, linux-arm-msm

On Mon 28 Sep 14:08 PDT 2015, Rob Clark wrote:

> On Mon, Sep 28, 2015 at 4:51 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 09/28, Rob Clark wrote:
> >> Add interfaces needed for configuring OCMEM.
> >>
> >> Signed-off-by: Rob Clark <robdclark@gmail.com>
> >> ---
> >>  drivers/firmware/qcom_scm-32.c |  57 ++++++++++++++++++++++
> >>  drivers/firmware/qcom_scm-64.c |  16 +++++++
> >>  drivers/firmware/qcom_scm.c    | 106 +++++++++++++++++++++++++++++++++++++++++
> >>  drivers/firmware/qcom_scm.h    |  13 +++++
> >>  include/linux/qcom_scm.h       |  10 ++++
> >>  5 files changed, 202 insertions(+)
> >>
> >> diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
[..]
> >> +bool qcom_scm_ocmem_secure_available(void)
> >> +{
> >> +     int ret = qcom_scm_clk_enable();
> >
> > I doubt we need to enable clocks to figure out if a call is
> > available. Please drop clk stuff here.
> 
> hmm, hdcp did, but pas didn't..  otoh it looks like the call to
> __qcom_scm_pas_supported() *should* be wrapped in clk enable/disable..
> 
> And __qcom_scm_is_call_available() does call qcom_scm_call().  Which
> is, I assume, what needs the clk's..  so not entirely sure if *all*
> the clk enable/disable stuff should be stripped out, or if missing clk
> stuff should be added in qcom_scm_pas_supported()..
> 

The scm clocks here are the crypto engine clocks, they are not needed to
check if TZ implements PAS for a given processor or not.

But it could be argued that this is simply an assumption I make of the
black box we're calling into...

> 
> >> +
> >> +     if (ret)
> >> +             goto clk_err;
> >> +
> >> +     ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SECURE_SVC,
> >> +                     QCOM_SCM_OCMEM_SECURE_CFG);
> >> +
> >> +     qcom_scm_clk_disable();
> >> +
> >> +clk_err:
> >> +     return (ret > 0) ? true : false;
> >> +}
> >> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_available);
> >> +
[..]
> >> +int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
> >> +{
> >> +     int ret = qcom_scm_clk_enable();
> >> +
> >> +     if (ret)
> >> +             return ret;
> >> +
> >> +     ret = __qcom_scm_ocmem_unlock(id, offset, size);
> >> +     qcom_scm_clk_disable();
> >> +
> >> +     return ret;
> >> +}
> >> +EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
> >
> > I don't think we need any clocks for lock/unlock/cfg either. The
> > scm clocks are some crypto clocks that the secure side isn't able
> > to enable and we don't have a device in DT for them. In the ocmem
> > case, we should rely on the ocmem device to get the clocks and
> > turn them on before calling any scm APIs that may require those
> > clocks.
> 
> Hmm, if that is true then we should probably drop the clks for hdcp
> fxns too, and maybe add a comment somewhere since it isn't really
> clear what the clks are for (and when it is unclear, folks will just
> cargo-cult what the existing fxns are doing).  As-is it is hard to
> tell what is required and what is luck..
> 

I would expect hdcp to use the crypto engines in some way and we don't
want people to feel that they should add the random clocks here, so
commenting them is probably the way to go.

> >> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> >> index 46d41e4..a934457 100644
> >> --- a/include/linux/qcom_scm.h
> >> +++ b/include/linux/qcom_scm.h
> >> @@ -23,10 +23,20 @@ struct qcom_scm_hdcp_req {
> >>       u32 val;
> >>  };
> >>
> >> +extern bool qcom_scm_is_available(void);
> >
> > Is this used? Looks like noise.
> 
> perhaps should be split out into a separate patch..  but I am using
> this, and it seems like a good idea to avoid null ptr deref's of
> __scm.  Probably some of the scm callers should call this first..
> either that or we should make other scm entry points behave better if
> __scm is null..
> 

This is part of Andy's platformication, didn't he export it properly?
I use it as well from the remoteproc.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] drm/msm: add OCMEM driver
  2015-09-28 18:51 ` [PATCH 4/4] drm/msm: add OCMEM driver Rob Clark
@ 2015-09-28 22:10   ` Stephen Boyd
  2015-09-28 22:53     ` Rob Clark
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Boyd @ 2015-09-28 22:10 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-arm-msm, Bjorn Andersson

On 09/28, Rob Clark wrote:
> @@ -322,10 +319,8 @@ static void a3xx_destroy(struct msm_gpu *gpu)
>  
>  	adreno_gpu_cleanup(adreno_gpu);
>  
> -#ifdef CONFIG_MSM_OCMEM
>  	if (a3xx_gpu->ocmem_base)

Is this supposed to be ocmem_base or ocmem_hdl? Perhaps this
check could be put inside the ocmem_free() itself so that the
caller doesn't have to care.

>  		ocmem_free(OCMEM_GRAPHICS, a3xx_gpu->ocmem_hdl);
> -#endif
>  
>  	kfree(a3xx_gpu);
>  }
> @@ -289,10 +288,8 @@ static void a4xx_destroy(struct msm_gpu *gpu)
>  
>  	adreno_gpu_cleanup(adreno_gpu);
>  
> -#ifdef CONFIG_MSM_OCMEM
> -	if (a4xx_gpu->ocmem_base)
> +	if (a4xx_gpu->ocmem_hdl)

This one changed, so a3xx above seems highly suspicious.

>  		ocmem_free(OCMEM_GRAPHICS, a4xx_gpu->ocmem_hdl);
> -#endif
>  
>  	kfree(a4xx_gpu);
>  }
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 2bbe85a..f042ba8 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -172,4 +172,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev);
>  void __init adreno_register(void);
>  void __exit adreno_unregister(void);
>  
> +void __init ocmem_register(void);
> +void __exit ocmem_unregister(void);

__init and __exit in header files is useless

> +
>  #endif /* __MSM_GPU_H__ */
> diff --git a/drivers/gpu/drm/msm/ocmem/ocmem.c b/drivers/gpu/drm/msm/ocmem/ocmem.c
> new file mode 100644
> index 0000000..d3cdd64
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/ocmem/ocmem.c
> @@ -0,0 +1,396 @@
> +/*
> + * Copyright (C) 2015 Red Hat
> + * Author: Rob Clark <robdclark@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/cpuset.h>

What is this include for?

> +#include <linux/qcom_scm.h>
> +
> +#include "msm_drv.h"
> +#include "ocmem.h"
> +#include "ocmem.xml.h"
> +
> +enum region_mode {
> +	WIDE_MODE = 0x0,
> +	THIN_MODE,
> +	MODE_DEFAULT = WIDE_MODE,
> +};
> +
> +enum ocmem_tz_client {
> +	TZ_UNUSED = 0x0,
> +	TZ_GRAPHICS,
> +	TZ_VIDEO,
> +	TZ_LP_AUDIO,
> +	TZ_SENSORS,
> +	TZ_OTHER_OS,
> +	TZ_DEBUG,
> +};
> +
> +struct ocmem_region {
> +	unsigned psgsc_ctrl;
> +	bool interleaved;
> +	enum region_mode mode;
> +	unsigned int num_macros;
> +	enum ocmem_macro_state macro_state[4];
> +	unsigned long macro_size;
> +	unsigned long region_size;
> +};
> +
> +struct ocmem_config {
> +	uint8_t  num_regions;
> +	uint32_t macro_size;
> +};
> +
> +struct ocmem {
> +	struct device *dev;
> +	const struct ocmem_config *config;
> +	struct resource *ocmem_mem;
> +	struct clk *core_clk;
> +	struct clk *iface_clk;
> +	void __iomem *mmio;
> +
> +	unsigned num_ports;

Is this used after probe?

> +	unsigned num_macros;
> +	bool interleaved;

Is this used after probe?

> +
> +	struct ocmem_region *regions;
> +};
> +
> +struct ocmem *ocmem;

static?

> +
> +static bool ocmem_exists(void);
> +
> +static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
> +{
> +	msm_writel(data, ocmem->mmio + reg);
> +}
> +
> +static inline u32 ocmem_read(struct ocmem *ocmem, u32 reg)
> +{
> +	return msm_readl(ocmem->mmio + reg);
> +}
> +
> +static int ocmem_clk_enable(struct ocmem *ocmem)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(ocmem->core_clk);
> +	if (ret)
> +		return ret;
> +
> +	if (ocmem->iface_clk) {
> +		ret = clk_prepare_enable(ocmem->iface_clk);

clk_prepare_enable() on NULL does nothing so it should be safe to
drop the if.

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void update_ocmem(struct ocmem *ocmem)
> +{
> +	uint32_t region_mode_ctrl = 0x0;
> +	unsigned pos = 0;
> +	unsigned i = 0;
> +
> +	if (!qcom_scm_ocmem_lock_available()) {
> +		for (i = 0; i < ocmem->config->num_regions; i++) {
> +			struct ocmem_region *region = &ocmem->regions[i];
> +			pos = i << 2;
> +			if (region->mode == THIN_MODE)
> +				region_mode_ctrl |= BIT(pos);
> +		}
> +		dev_dbg(ocmem->dev, "ocmem_region_mode_control %x\n", region_mode_ctrl);
> +		ocmem_write(ocmem, REG_OCMEM_REGION_MODE_CTL, region_mode_ctrl);
> +		/* Barrier to commit the region mode */
> +		mb();

msm_writel() already has a barrier, so now we have a double
barrier?

> +	}
> +
> +	for (i = 0; i < ocmem->config->num_regions; i++) {
> +		struct ocmem_region *region = &ocmem->regions[i];
> +
> +		ocmem_write(ocmem, REG_OCMEM_PSGSC_CTL(i),
> +				OCMEM_PSGSC_CTL_MACRO0_MODE(region->macro_state[0]) |
> +				OCMEM_PSGSC_CTL_MACRO1_MODE(region->macro_state[1]) |
> +				OCMEM_PSGSC_CTL_MACRO2_MODE(region->macro_state[2]) |
> +				OCMEM_PSGSC_CTL_MACRO3_MODE(region->macro_state[3]));
> +	}
> +}
> +
> +inline unsigned long phys_to_offset(unsigned long addr)

static? drop inline?

> +{
> +	if ((addr < ocmem->ocmem_mem->start) ||
> +		(addr >= ocmem->ocmem_mem->end))
> +		return 0;
> +	return addr - ocmem->ocmem_mem->start;
> +}
> +
> +static unsigned long device_address(enum ocmem_client client, unsigned long addr)

client is not used, so remove it?

> +{
> +	/* TODO, gpu uses phys_to_offset, but others do not.. */
> +	return phys_to_offset(addr);
> +}
> +
> +static void update_range(struct ocmem *ocmem, struct ocmem_buf *buf,
> +		enum ocmem_macro_state mstate, enum region_mode rmode)
> +{
> +	unsigned long offset = 0;
> +	int i, j;
> +
> +	/*
> +	 * TODO probably should assert somewhere that range is aligned
> +	 * to macro boundaries..
> +	 */
> +
> +	for (i = 0; i < ocmem->config->num_regions; i++) {
> +		struct ocmem_region *region = &ocmem->regions[i];
> +		if ((buf->offset <= offset) && (offset < (buf->offset + buf->len)))

useless parentheses here.

> +			region->mode = rmode;
> +		for (j = 0; j < region->num_macros; j++) {
> +			if ((buf->offset <= offset) && (offset < (buf->offset + buf->len)))
> +				region->macro_state[j] = mstate;
> +			offset += region->macro_size;
> +		}
> +	}
> +
> +	update_ocmem(ocmem);
> +}
> +
> +struct ocmem_buf *ocmem_allocate(enum ocmem_client client, unsigned long size)
> +{
> +	struct ocmem_buf *buf;
> +
> +	if (!ocmem) {
> +		if (ocmem_exists())

Does this ever trigger? From what I can tell the only place
ocmem_allocate() is called from is the open path of the gpu
device node, and that shouldn't happen until ocmem and gpu
drivers have both probed, in which case ocmem is already
non-NULLL or it never will exist so we should return ENXIO
without searching.

> +			return ERR_PTR(-EPROBE_DEFER);
> +		return ERR_PTR(-ENXIO);
> +	}
> +
> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);

if (!buf)?

> +
> +	/*
> +	 * TODO less hard-coded allocation that works for more than
> +	 * one user:
> +	 */
> +
> +	buf->offset = 0;
> +	buf->addr = device_address(client, buf->offset);
> +	buf->len = size;
> +
> +	update_range(ocmem, buf, CORE_ON, WIDE_MODE);
> +
> +	if (qcom_scm_ocmem_lock_available()) {
> +		int ret;
> +		ret = qcom_scm_ocmem_lock(TZ_GRAPHICS, buf->offset, buf->len,
> +				WIDE_MODE);
> +		if (ret)
> +			dev_err(ocmem->dev, "could not lock: %d\n", ret);
> +	} else {
> +		if (client == OCMEM_GRAPHICS) {
> +			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_START, buf->offset);
> +			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_END, buf->offset + buf->len);
> +		}
> +	}
> +
> +	return buf;
> +}
> +
> +void ocmem_free(enum ocmem_client client, struct ocmem_buf *buf)
> +{
> +	update_range(ocmem, buf, CLK_OFF, MODE_DEFAULT);
> +
> +	if (qcom_scm_ocmem_lock_available()) {
> +		int ret;
> +		ret = qcom_scm_ocmem_unlock(TZ_GRAPHICS, buf->offset, buf->len);
> +		if (ret)
> +			dev_err(ocmem->dev, "could not lock: %d\n", ret);

could not unlock?

> +	} else {
> +		if (client == OCMEM_GRAPHICS) {
> +			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_START, 0x0);
> +			ocmem_write(ocmem, REG_OCMEM_GFX_MPU_END, 0x0);
> +		}
> +	}
> +
> +	kfree(buf);
> +}
> +
> +static const struct ocmem_config ocmem_8974_config = {
> +	.num_regions = 3, .macro_size = SZ_128K,
> +};
> +
> +static const struct of_device_id dt_match[] = {

Perhaps ocmem_dt_match? There's probably quite a few dt_match
arrays out there.

> +	{ .compatible = "qcom,msm-ocmem-8974", .data = &ocmem_8974_config },

Is there a binding for this somewhere? I'd prefer we move msm
int the name to the numbers part and call it qcom,ocmem-msm8974.

> +	{}
> +};
> +
> +static int ocmem_dev_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct ocmem_config *config = NULL;
> +	uint32_t reg, num_banks, region_size;
> +	int i, j, ret;
> +
> +	struct device_node *of_node = dev->of_node;
> +	const struct of_device_id *match;
> +
> +	match = of_match_node(dt_match, dev->of_node);
> +	if (match)
> +		config = match->data;
> +
> +	if (!config) {

Just use of_match_device() instead.

> +		dev_err(dev, "unknown config: %s\n", of_node->name);
> +		return -ENXIO;
> +	}
> +
> +	ocmem = devm_kzalloc(dev, sizeof(*ocmem), GFP_KERNEL);
> +	if (!ocmem)
> +		return -ENOMEM;
> +
> +	ocmem->dev = dev;
> +	ocmem->config = config;
> +
> +	ocmem->core_clk = devm_clk_get(dev, "core_clk");
> +	if (IS_ERR(ocmem->core_clk)) {
> +		dev_err(dev, "Unable to get the core clock\n");

Maybe we should be silent, in case this returns a probe defer
error.

> +		return PTR_ERR(ocmem->core_clk);
> +	}
> +
> +	ocmem->iface_clk = devm_clk_get(dev, "iface_clk");
> +	if (IS_ERR_OR_NULL(ocmem->iface_clk))

This should make sure it isn't a probe defer error.

> +		ocmem->iface_clk = NULL;
> +
> +	/* The core clock is synchronous with graphics */
> +	WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
> +
> +	ocmem->mmio = msm_ioremap(pdev, "ocmem_ctrl_physical", "OCMEM");
> +	if (IS_ERR(ocmem->mmio))
> +		return PTR_ERR(ocmem->mmio);
> +
> +	ocmem->ocmem_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +			"ocmem_physical");
> +	if (!ocmem->ocmem_mem) {
> +		dev_err(dev, "could not get OCMEM region\n");
> +		return -ENXIO;
> +	}
> +
> +	ret = ocmem_clk_enable(ocmem);
> +	if (ret)
> +		goto fail;
> +
> +	if (qcom_scm_is_available() && qcom_scm_ocmem_secure_available()) {
> +		dev_info(dev, "configuring scm\n");

debug noise?

> +		ret = qcom_scm_ocmem_secure_cfg(0x5);
> +		if (ret)
> +			goto fail;
> +	}
> +
> +	reg = ocmem_read(ocmem, REG_OCMEM_HW_PROFILE);
> +	ocmem->num_ports = FIELD(reg, OCMEM_HW_PROFILE_NUM_PORTS);
> +	ocmem->num_macros = FIELD(reg, OCMEM_HW_PROFILE_NUM_MACROS);
> +	ocmem->interleaved = !!(reg & OCMEM_HW_PROFILE_INTERLEAVING);
> +
> +	num_banks = ocmem->num_ports / 2;
> +	region_size = config->macro_size * num_banks;
> +
> +	dev_info(dev, "%u ports, %u regions, %u macros, %sinterleaved\n",
> +			ocmem->num_ports, config->num_regions, ocmem->num_macros,
> +			ocmem->interleaved ? "" : "not ");
> +
> +	ocmem->regions = devm_kzalloc(dev, sizeof(struct ocmem_region) *
> +			config->num_regions, GFP_KERNEL);

devm_kcalloc()?

> +	if (!ocmem->regions) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	for (i = 0; i < config->num_regions; i++) {
> +		struct ocmem_region *region = &ocmem->regions[i];
> +
> +		if (WARN_ON(num_banks > ARRAY_SIZE(region->macro_state))) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +
> +		region->mode = MODE_DEFAULT;
> +		region->num_macros = num_banks;
> +
> +		if ((i == (config->num_regions - 1)) &&

One too many parentheses here.

> +				(reg & OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE)) {
> +			region->macro_size = config->macro_size / 2;
> +			region->region_size = region_size / 2;
> +		} else {
> +			region->macro_size = config->macro_size;
> +			region->region_size = region_size;
> +		}
> +
> +		for (j = 0; j < ARRAY_SIZE(region->macro_state); j++)
> +			region->macro_state[j] = CLK_OFF;
> +	}
> +
> +	return 0;
> +
> +fail:
> +	dev_err(dev, "probe failed\n");

debug noise?

> +	ocmem_dev_remove(pdev);
> +	return ret;
> +}
> +
> +static struct platform_driver ocmem_driver = {
> +	.probe = ocmem_dev_probe,
> +	.remove = ocmem_dev_remove,
> +	.driver = {
> +		.name = "ocmem",
> +		.of_match_table = dt_match,
> +	},
> +};

Does this need a module table?

> +
> +static bool ocmem_exists(void)
> +{
> +	struct device_driver *drv = &ocmem_driver.driver;
> +	struct device *d;
> +
> +	d = bus_find_device(&platform_bus_type, NULL, drv,
> +			(void *)platform_bus_type.match);
> +	if (d) {
> +		put_device(d);
> +		return true;
> +	}
> +
> +	return false;
> +}

I hope this function isn't necessary.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] qcom-scm: add ocmem support
  2015-09-28 21:59       ` Bjorn Andersson
@ 2015-09-28 22:35         ` Stephen Boyd
  2015-09-28 23:02           ` Rob Clark
  0 siblings, 1 reply; 13+ messages in thread
From: Stephen Boyd @ 2015-09-28 22:35 UTC (permalink / raw)
  To: Bjorn Andersson; +Cc: Rob Clark, dri-devel, linux-arm-msm

On 09/28, Bjorn Andersson wrote:
> On Mon 28 Sep 14:08 PDT 2015, Rob Clark wrote:
> 
> > On Mon, Sep 28, 2015 at 4:51 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > > On 09/28, Rob Clark wrote:
> > >> +bool qcom_scm_ocmem_secure_available(void)
> > >> +{
> > >> +     int ret = qcom_scm_clk_enable();
> > >
> > > I doubt we need to enable clocks to figure out if a call is
> > > available. Please drop clk stuff here.
> > 
> > hmm, hdcp did, but pas didn't..  otoh it looks like the call to
> > __qcom_scm_pas_supported() *should* be wrapped in clk enable/disable..
> > 
> > And __qcom_scm_is_call_available() does call qcom_scm_call().  Which
> > is, I assume, what needs the clk's..  so not entirely sure if *all*
> > the clk enable/disable stuff should be stripped out, or if missing clk
> > stuff should be added in qcom_scm_pas_supported()..
> > 
> 
> The scm clocks here are the crypto engine clocks, they are not needed to
> check if TZ implements PAS for a given processor or not.
> 
> But it could be argued that this is simply an assumption I make of the
> black box we're calling into...

Let's not make assumptions. They're not needed to check if it has
support for something.

> 
> > 
> > >> +
> > >> +     if (ret)
> > >> +             goto clk_err;
> > >> +
> > >> +     ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SECURE_SVC,
> > >> +                     QCOM_SCM_OCMEM_SECURE_CFG);
> > >> +
> > >> +     qcom_scm_clk_disable();
> > >> +
> > >> +clk_err:
> > >> +     return (ret > 0) ? true : false;
> > >> +}
> > >> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_available);
> > >> +
> [..]
> > >> +int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
> > >> +{
> > >> +     int ret = qcom_scm_clk_enable();
> > >> +
> > >> +     if (ret)
> > >> +             return ret;
> > >> +
> > >> +     ret = __qcom_scm_ocmem_unlock(id, offset, size);
> > >> +     qcom_scm_clk_disable();
> > >> +
> > >> +     return ret;
> > >> +}
> > >> +EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
> > >
> > > I don't think we need any clocks for lock/unlock/cfg either. The
> > > scm clocks are some crypto clocks that the secure side isn't able
> > > to enable and we don't have a device in DT for them. In the ocmem
> > > case, we should rely on the ocmem device to get the clocks and
> > > turn them on before calling any scm APIs that may require those
> > > clocks.
> > 
> > Hmm, if that is true then we should probably drop the clks for hdcp
> > fxns too, and maybe add a comment somewhere since it isn't really
> > clear what the clks are for (and when it is unclear, folks will just
> > cargo-cult what the existing fxns are doing).  As-is it is hard to
> > tell what is required and what is luck..
> > 
> 
> I would expect hdcp to use the crypto engines in some way and we don't
> want people to feel that they should add the random clocks here, so
> commenting them is probably the way to go.

Yes HDCP uses crypto for something so those clock calls should
stay. If the clocks were used by a HDCP device then it would be
like the ocmem case, but it isn't.

> 
> > >> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> > >> index 46d41e4..a934457 100644
> > >> --- a/include/linux/qcom_scm.h
> > >> +++ b/include/linux/qcom_scm.h
> > >> @@ -23,10 +23,20 @@ struct qcom_scm_hdcp_req {
> > >>       u32 val;
> > >>  };
> > >>
> > >> +extern bool qcom_scm_is_available(void);
> > >
> > > Is this used? Looks like noise.
> > 
> > perhaps should be split out into a separate patch..  but I am using
> > this, and it seems like a good idea to avoid null ptr deref's of
> > __scm.  Probably some of the scm callers should call this first..
> > either that or we should make other scm entry points behave better if
> > __scm is null..
> > 
> 
> This is part of Andy's platformication, didn't he export it properly?
> I use it as well from the remoteproc.

Do we probe defer ocmem if scm isn't ready? Maybe we should name
it qcom_scm_is_probed() and have it return -EPROBE_DEFER if it
isn't probed and 0 if it is probed. Then drivers just call that
function and return the error if there is one. I'd rather not
litter scm_*() APIs with checks to see if the driver has probed
yet. Just let those crash the system. Of course, this probably
doesn't matter because we don't need to do any clock stuff here
anyway.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] drm/msm: add OCMEM driver
  2015-09-28 22:10   ` Stephen Boyd
@ 2015-09-28 22:53     ` Rob Clark
  2015-09-29  1:58       ` Stephen Boyd
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Clark @ 2015-09-28 22:53 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: dri-devel, linux-arm-msm, Bjorn Andersson

On Mon, Sep 28, 2015 at 6:10 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 09/28, Rob Clark wrote:
>> @@ -322,10 +319,8 @@ static void a3xx_destroy(struct msm_gpu *gpu)
>>
>>       adreno_gpu_cleanup(adreno_gpu);
>>
>> -#ifdef CONFIG_MSM_OCMEM
>>       if (a3xx_gpu->ocmem_base)
>
> Is this supposed to be ocmem_base or ocmem_hdl? Perhaps this
> check could be put inside the ocmem_free() itself so that the
> caller doesn't have to care.

yeah, should be ocmem_hdl

I would kind of prefer to keep the check for null in the caller, just
to simplify backports to 3.10 kernel (since otherwise the API matches
downstream).. Although I guess downstream checks for null and spits
out error msg and returns -EINVAL, so maybe that is enough..

>>               ocmem_free(OCMEM_GRAPHICS, a3xx_gpu->ocmem_hdl);
>> -#endif
>>
>>       kfree(a3xx_gpu);
>>  }
>> @@ -289,10 +288,8 @@ static void a4xx_destroy(struct msm_gpu *gpu)
>>
>>       adreno_gpu_cleanup(adreno_gpu);
>>
>> -#ifdef CONFIG_MSM_OCMEM
>> -     if (a4xx_gpu->ocmem_base)
>> +     if (a4xx_gpu->ocmem_hdl)
>
> This one changed, so a3xx above seems highly suspicious.
>
>>               ocmem_free(OCMEM_GRAPHICS, a4xx_gpu->ocmem_hdl);
>> -#endif
>>
>>       kfree(a4xx_gpu);
>>  }
>> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
>> index 2bbe85a..f042ba8 100644
>> --- a/drivers/gpu/drm/msm/msm_gpu.h
>> +++ b/drivers/gpu/drm/msm/msm_gpu.h
>> @@ -172,4 +172,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev);
>>  void __init adreno_register(void);
>>  void __exit adreno_unregister(void);
>>
>> +void __init ocmem_register(void);
>> +void __exit ocmem_unregister(void);
>
> __init and __exit in header files is useless
>
>> +
>>  #endif /* __MSM_GPU_H__ */
>> diff --git a/drivers/gpu/drm/msm/ocmem/ocmem.c b/drivers/gpu/drm/msm/ocmem/ocmem.c
>> new file mode 100644
>> index 0000000..d3cdd64
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/ocmem/ocmem.c
>> @@ -0,0 +1,396 @@
>> +/*
>> + * Copyright (C) 2015 Red Hat
>> + * Author: Rob Clark <robdclark@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/cpuset.h>
>
> What is this include for?

needed for qcom_scm.h, although I guess I could just add the missing
#includes in qcom_scm.h instead..

>> +#include <linux/qcom_scm.h>
>> +
>> +#include "msm_drv.h"
>> +#include "ocmem.h"
>> +#include "ocmem.xml.h"
>> +
>> +enum region_mode {
>> +     WIDE_MODE = 0x0,
>> +     THIN_MODE,
>> +     MODE_DEFAULT = WIDE_MODE,
>> +};
>> +
>> +enum ocmem_tz_client {
>> +     TZ_UNUSED = 0x0,
>> +     TZ_GRAPHICS,
>> +     TZ_VIDEO,
>> +     TZ_LP_AUDIO,
>> +     TZ_SENSORS,
>> +     TZ_OTHER_OS,
>> +     TZ_DEBUG,
>> +};
>> +
>> +struct ocmem_region {
>> +     unsigned psgsc_ctrl;
>> +     bool interleaved;
>> +     enum region_mode mode;
>> +     unsigned int num_macros;
>> +     enum ocmem_macro_state macro_state[4];
>> +     unsigned long macro_size;
>> +     unsigned long region_size;
>> +};
>> +
>> +struct ocmem_config {
>> +     uint8_t  num_regions;
>> +     uint32_t macro_size;
>> +};
>> +
>> +struct ocmem {
>> +     struct device *dev;
>> +     const struct ocmem_config *config;
>> +     struct resource *ocmem_mem;
>> +     struct clk *core_clk;
>> +     struct clk *iface_clk;
>> +     void __iomem *mmio;
>> +
>> +     unsigned num_ports;
>
> Is this used after probe?

not currently.. downstream was saving it off in pdata but on closer
look it doesn't seem to use it after probe either..

>> +     unsigned num_macros;
>> +     bool interleaved;
>
> Is this used after probe?

again, cargo culted from downstream, but it looks like we can drop..

>> +
>> +     struct ocmem_region *regions;
>> +};
>> +
>> +struct ocmem *ocmem;
>
> static?
>
>> +
>> +static bool ocmem_exists(void);
>> +
>> +static inline void ocmem_write(struct ocmem *ocmem, u32 reg, u32 data)
>> +{
>> +     msm_writel(data, ocmem->mmio + reg);
>> +}
>> +
>> +static inline u32 ocmem_read(struct ocmem *ocmem, u32 reg)
>> +{
>> +     return msm_readl(ocmem->mmio + reg);
>> +}
>> +
>> +static int ocmem_clk_enable(struct ocmem *ocmem)
>> +{
>> +     int ret;
>> +
>> +     ret = clk_prepare_enable(ocmem->core_clk);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (ocmem->iface_clk) {
>> +             ret = clk_prepare_enable(ocmem->iface_clk);
>
> clk_prepare_enable() on NULL does nothing so it should be safe to
> drop the if.
>
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static void update_ocmem(struct ocmem *ocmem)
>> +{
>> +     uint32_t region_mode_ctrl = 0x0;
>> +     unsigned pos = 0;
>> +     unsigned i = 0;
>> +
>> +     if (!qcom_scm_ocmem_lock_available()) {
>> +             for (i = 0; i < ocmem->config->num_regions; i++) {
>> +                     struct ocmem_region *region = &ocmem->regions[i];
>> +                     pos = i << 2;
>> +                     if (region->mode == THIN_MODE)
>> +                             region_mode_ctrl |= BIT(pos);
>> +             }
>> +             dev_dbg(ocmem->dev, "ocmem_region_mode_control %x\n", region_mode_ctrl);
>> +             ocmem_write(ocmem, REG_OCMEM_REGION_MODE_CTL, region_mode_ctrl);
>> +             /* Barrier to commit the region mode */
>> +             mb();
>
> msm_writel() already has a barrier, so now we have a double
> barrier?

hmm, msm_writel() doesn't have any more barrier than writel().. so I
kept the mb() from downstream..

>> +     }
>> +
>> +     for (i = 0; i < ocmem->config->num_regions; i++) {
>> +             struct ocmem_region *region = &ocmem->regions[i];
>> +
>> +             ocmem_write(ocmem, REG_OCMEM_PSGSC_CTL(i),
>> +                             OCMEM_PSGSC_CTL_MACRO0_MODE(region->macro_state[0]) |
>> +                             OCMEM_PSGSC_CTL_MACRO1_MODE(region->macro_state[1]) |
>> +                             OCMEM_PSGSC_CTL_MACRO2_MODE(region->macro_state[2]) |
>> +                             OCMEM_PSGSC_CTL_MACRO3_MODE(region->macro_state[3]));
>> +     }
>> +}
>> +
>> +inline unsigned long phys_to_offset(unsigned long addr)
>
> static? drop inline?
>
>> +{
>> +     if ((addr < ocmem->ocmem_mem->start) ||
>> +             (addr >= ocmem->ocmem_mem->end))
>> +             return 0;
>> +     return addr - ocmem->ocmem_mem->start;
>> +}
>> +
>> +static unsigned long device_address(enum ocmem_client client, unsigned long addr)
>
> client is not used, so remove it?

client would be used if we supported more than just gfx, so I left
this param in case some day someone wants to support more than just
gfx..

>> +{
>> +     /* TODO, gpu uses phys_to_offset, but others do not.. */
>> +     return phys_to_offset(addr);
>> +}
>> +
>> +static void update_range(struct ocmem *ocmem, struct ocmem_buf *buf,
>> +             enum ocmem_macro_state mstate, enum region_mode rmode)
>> +{
>> +     unsigned long offset = 0;
>> +     int i, j;
>> +
>> +     /*
>> +      * TODO probably should assert somewhere that range is aligned
>> +      * to macro boundaries..
>> +      */
>> +
>> +     for (i = 0; i < ocmem->config->num_regions; i++) {
>> +             struct ocmem_region *region = &ocmem->regions[i];
>> +             if ((buf->offset <= offset) && (offset < (buf->offset + buf->len)))
>
> useless parentheses here.

yes, but I prefer them ;-)

>> +                     region->mode = rmode;
>> +             for (j = 0; j < region->num_macros; j++) {
>> +                     if ((buf->offset <= offset) && (offset < (buf->offset + buf->len)))
>> +                             region->macro_state[j] = mstate;
>> +                     offset += region->macro_size;
>> +             }
>> +     }
>> +
>> +     update_ocmem(ocmem);
>> +}
>> +
>> +struct ocmem_buf *ocmem_allocate(enum ocmem_client client, unsigned long size)
>> +{
>> +     struct ocmem_buf *buf;
>> +
>> +     if (!ocmem) {
>> +             if (ocmem_exists())
>
> Does this ever trigger? From what I can tell the only place
> ocmem_allocate() is called from is the open path of the gpu
> device node, and that shouldn't happen until ocmem and gpu
> drivers have both probed, in which case ocmem is already
> non-NULLL or it never will exist so we should return ENXIO
> without searching.

this was mostly just planning ahead for other users and/or moving
ocmem out of drm/msm (ie. if it was a loadable module)

>> +                     return ERR_PTR(-EPROBE_DEFER);
>> +             return ERR_PTR(-ENXIO);
>> +     }
>> +
>> +     buf = kzalloc(sizeof(*buf), GFP_KERNEL);
>
> if (!buf)?
>
>> +
>> +     /*
>> +      * TODO less hard-coded allocation that works for more than
>> +      * one user:
>> +      */
>> +
>> +     buf->offset = 0;
>> +     buf->addr = device_address(client, buf->offset);
>> +     buf->len = size;
>> +
>> +     update_range(ocmem, buf, CORE_ON, WIDE_MODE);
>> +
>> +     if (qcom_scm_ocmem_lock_available()) {
>> +             int ret;
>> +             ret = qcom_scm_ocmem_lock(TZ_GRAPHICS, buf->offset, buf->len,
>> +                             WIDE_MODE);
>> +             if (ret)
>> +                     dev_err(ocmem->dev, "could not lock: %d\n", ret);
>> +     } else {
>> +             if (client == OCMEM_GRAPHICS) {
>> +                     ocmem_write(ocmem, REG_OCMEM_GFX_MPU_START, buf->offset);
>> +                     ocmem_write(ocmem, REG_OCMEM_GFX_MPU_END, buf->offset + buf->len);
>> +             }
>> +     }
>> +
>> +     return buf;
>> +}
>> +
>> +void ocmem_free(enum ocmem_client client, struct ocmem_buf *buf)
>> +{
>> +     update_range(ocmem, buf, CLK_OFF, MODE_DEFAULT);
>> +
>> +     if (qcom_scm_ocmem_lock_available()) {
>> +             int ret;
>> +             ret = qcom_scm_ocmem_unlock(TZ_GRAPHICS, buf->offset, buf->len);
>> +             if (ret)
>> +                     dev_err(ocmem->dev, "could not lock: %d\n", ret);
>
> could not unlock?
>
>> +     } else {
>> +             if (client == OCMEM_GRAPHICS) {
>> +                     ocmem_write(ocmem, REG_OCMEM_GFX_MPU_START, 0x0);
>> +                     ocmem_write(ocmem, REG_OCMEM_GFX_MPU_END, 0x0);
>> +             }
>> +     }
>> +
>> +     kfree(buf);
>> +}
>> +
>> +static const struct ocmem_config ocmem_8974_config = {
>> +     .num_regions = 3, .macro_size = SZ_128K,
>> +};
>> +
>> +static const struct of_device_id dt_match[] = {
>
> Perhaps ocmem_dt_match? There's probably quite a few dt_match
> arrays out there.
>
>> +     { .compatible = "qcom,msm-ocmem-8974", .data = &ocmem_8974_config },
>
> Is there a binding for this somewhere? I'd prefer we move msm
> int the name to the numbers part and call it qcom,ocmem-msm8974.

sure, I could change that..  I was a bit lazy about including bindings
doc, although right now it is purely generic binding, ie:

    qcom,msm-ocmem-8974@fdd00000 {
        compatible = "qcom,ocmem-msm8974";
        reg-names =
            "ocmem_ctrl_physical",
            "ocmem_physical";
        reg =
            <0xfdd00000 0x2000>,
            <0xfec00000 0x180000>;
        clock-names =
            "core_clk",
            "iface_clk";
        clocks =
            <&rpmcc RPM_OCMEMGX_A_CLK>,
            <&mmcc OCMEMCX_OCMEMNOC_CLK>;
    };


>> +     {}
>> +};
>> +
>> +static int ocmem_dev_probe(struct platform_device *pdev)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     const struct ocmem_config *config = NULL;
>> +     uint32_t reg, num_banks, region_size;
>> +     int i, j, ret;
>> +
>> +     struct device_node *of_node = dev->of_node;
>> +     const struct of_device_id *match;
>> +
>> +     match = of_match_node(dt_match, dev->of_node);
>> +     if (match)
>> +             config = match->data;
>> +
>> +     if (!config) {
>
> Just use of_match_device() instead.
>
>> +             dev_err(dev, "unknown config: %s\n", of_node->name);
>> +             return -ENXIO;
>> +     }
>> +
>> +     ocmem = devm_kzalloc(dev, sizeof(*ocmem), GFP_KERNEL);
>> +     if (!ocmem)
>> +             return -ENOMEM;
>> +
>> +     ocmem->dev = dev;
>> +     ocmem->config = config;
>> +
>> +     ocmem->core_clk = devm_clk_get(dev, "core_clk");
>> +     if (IS_ERR(ocmem->core_clk)) {
>> +             dev_err(dev, "Unable to get the core clock\n");
>
> Maybe we should be silent, in case this returns a probe defer
> error.

saves me adding debug prints later when debugging probe-defer fun ;-)

>> +             return PTR_ERR(ocmem->core_clk);
>> +     }
>> +
>> +     ocmem->iface_clk = devm_clk_get(dev, "iface_clk");
>> +     if (IS_ERR_OR_NULL(ocmem->iface_clk))
>
> This should make sure it isn't a probe defer error.

hmm, true, it is coming from a different clk driver compared to core_clk..

>> +             ocmem->iface_clk = NULL;
>> +
>> +     /* The core clock is synchronous with graphics */
>> +     WARN_ON(clk_set_rate(ocmem->core_clk, 1000) < 0);
>> +
>> +     ocmem->mmio = msm_ioremap(pdev, "ocmem_ctrl_physical", "OCMEM");
>> +     if (IS_ERR(ocmem->mmio))
>> +             return PTR_ERR(ocmem->mmio);
>> +
>> +     ocmem->ocmem_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> +                     "ocmem_physical");
>> +     if (!ocmem->ocmem_mem) {
>> +             dev_err(dev, "could not get OCMEM region\n");
>> +             return -ENXIO;
>> +     }
>> +
>> +     ret = ocmem_clk_enable(ocmem);
>> +     if (ret)
>> +             goto fail;
>> +
>> +     if (qcom_scm_is_available() && qcom_scm_ocmem_secure_available()) {
>> +             dev_info(dev, "configuring scm\n");
>
> debug noise?
>
>> +             ret = qcom_scm_ocmem_secure_cfg(0x5);
>> +             if (ret)
>> +                     goto fail;
>> +     }
>> +
>> +     reg = ocmem_read(ocmem, REG_OCMEM_HW_PROFILE);
>> +     ocmem->num_ports = FIELD(reg, OCMEM_HW_PROFILE_NUM_PORTS);
>> +     ocmem->num_macros = FIELD(reg, OCMEM_HW_PROFILE_NUM_MACROS);
>> +     ocmem->interleaved = !!(reg & OCMEM_HW_PROFILE_INTERLEAVING);
>> +
>> +     num_banks = ocmem->num_ports / 2;
>> +     region_size = config->macro_size * num_banks;
>> +
>> +     dev_info(dev, "%u ports, %u regions, %u macros, %sinterleaved\n",
>> +                     ocmem->num_ports, config->num_regions, ocmem->num_macros,
>> +                     ocmem->interleaved ? "" : "not ");
>> +
>> +     ocmem->regions = devm_kzalloc(dev, sizeof(struct ocmem_region) *
>> +                     config->num_regions, GFP_KERNEL);
>
> devm_kcalloc()?
>
>> +     if (!ocmem->regions) {
>> +             ret = -ENOMEM;
>> +             goto fail;
>> +     }
>> +
>> +     for (i = 0; i < config->num_regions; i++) {
>> +             struct ocmem_region *region = &ocmem->regions[i];
>> +
>> +             if (WARN_ON(num_banks > ARRAY_SIZE(region->macro_state))) {
>> +                     ret = -EINVAL;
>> +                     goto fail;
>> +             }
>> +
>> +             region->mode = MODE_DEFAULT;
>> +             region->num_macros = num_banks;
>> +
>> +             if ((i == (config->num_regions - 1)) &&
>
> One too many parentheses here.
>
>> +                             (reg & OCMEM_HW_PROFILE_LAST_REGN_HALFSIZE)) {
>> +                     region->macro_size = config->macro_size / 2;
>> +                     region->region_size = region_size / 2;
>> +             } else {
>> +                     region->macro_size = config->macro_size;
>> +                     region->region_size = region_size;
>> +             }
>> +
>> +             for (j = 0; j < ARRAY_SIZE(region->macro_state); j++)
>> +                     region->macro_state[j] = CLK_OFF;
>> +     }
>> +
>> +     return 0;
>> +
>> +fail:
>> +     dev_err(dev, "probe failed\n");
>
> debug noise?
>
>> +     ocmem_dev_remove(pdev);
>> +     return ret;
>> +}
>> +
>> +static struct platform_driver ocmem_driver = {
>> +     .probe = ocmem_dev_probe,
>> +     .remove = ocmem_dev_remove,
>> +     .driver = {
>> +             .name = "ocmem",
>> +             .of_match_table = dt_match,
>> +     },
>> +};
>
> Does this need a module table?

currently, no, since it is not split out into it's own .ko.. but it
would eventually when split out

(but iirc I'd end up w/ duplicate symbols issue if I added
MODULE_DEVICE_TABLE() currently)

>> +
>> +static bool ocmem_exists(void)
>> +{
>> +     struct device_driver *drv = &ocmem_driver.driver;
>> +     struct device *d;
>> +
>> +     d = bus_find_device(&platform_bus_type, NULL, drv,
>> +                     (void *)platform_bus_type.match);
>> +     if (d) {
>> +             put_device(d);
>> +             return true;
>> +     }
>> +
>> +     return false;
>> +}
>
> I hope this function isn't necessary.

currently, I don't think it would be.. but would be if split out of drm/msm..

BR,
-R


> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] qcom-scm: add ocmem support
  2015-09-28 22:35         ` Stephen Boyd
@ 2015-09-28 23:02           ` Rob Clark
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Clark @ 2015-09-28 23:02 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Bjorn Andersson, dri-devel, linux-arm-msm

On Mon, Sep 28, 2015 at 6:35 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 09/28, Bjorn Andersson wrote:
>> On Mon 28 Sep 14:08 PDT 2015, Rob Clark wrote:
>>
>> > On Mon, Sep 28, 2015 at 4:51 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > > On 09/28, Rob Clark wrote:
>> > >> +bool qcom_scm_ocmem_secure_available(void)
>> > >> +{
>> > >> +     int ret = qcom_scm_clk_enable();
>> > >
>> > > I doubt we need to enable clocks to figure out if a call is
>> > > available. Please drop clk stuff here.
>> >
>> > hmm, hdcp did, but pas didn't..  otoh it looks like the call to
>> > __qcom_scm_pas_supported() *should* be wrapped in clk enable/disable..
>> >
>> > And __qcom_scm_is_call_available() does call qcom_scm_call().  Which
>> > is, I assume, what needs the clk's..  so not entirely sure if *all*
>> > the clk enable/disable stuff should be stripped out, or if missing clk
>> > stuff should be added in qcom_scm_pas_supported()..
>> >
>>
>> The scm clocks here are the crypto engine clocks, they are not needed to
>> check if TZ implements PAS for a given processor or not.
>>
>> But it could be argued that this is simply an assumption I make of the
>> black box we're calling into...
>
> Let's not make assumptions. They're not needed to check if it has
> support for something.
>
>>
>> >
>> > >> +
>> > >> +     if (ret)
>> > >> +             goto clk_err;
>> > >> +
>> > >> +     ret = __qcom_scm_is_call_available(QCOM_SCM_OCMEM_SECURE_SVC,
>> > >> +                     QCOM_SCM_OCMEM_SECURE_CFG);
>> > >> +
>> > >> +     qcom_scm_clk_disable();
>> > >> +
>> > >> +clk_err:
>> > >> +     return (ret > 0) ? true : false;
>> > >> +}
>> > >> +EXPORT_SYMBOL(qcom_scm_ocmem_secure_available);
>> > >> +
>> [..]
>> > >> +int qcom_scm_ocmem_unlock(uint32_t id, uint32_t offset, uint32_t size)
>> > >> +{
>> > >> +     int ret = qcom_scm_clk_enable();
>> > >> +
>> > >> +     if (ret)
>> > >> +             return ret;
>> > >> +
>> > >> +     ret = __qcom_scm_ocmem_unlock(id, offset, size);
>> > >> +     qcom_scm_clk_disable();
>> > >> +
>> > >> +     return ret;
>> > >> +}
>> > >> +EXPORT_SYMBOL(qcom_scm_ocmem_unlock);
>> > >
>> > > I don't think we need any clocks for lock/unlock/cfg either. The
>> > > scm clocks are some crypto clocks that the secure side isn't able
>> > > to enable and we don't have a device in DT for them. In the ocmem
>> > > case, we should rely on the ocmem device to get the clocks and
>> > > turn them on before calling any scm APIs that may require those
>> > > clocks.
>> >
>> > Hmm, if that is true then we should probably drop the clks for hdcp
>> > fxns too, and maybe add a comment somewhere since it isn't really
>> > clear what the clks are for (and when it is unclear, folks will just
>> > cargo-cult what the existing fxns are doing).  As-is it is hard to
>> > tell what is required and what is luck..
>> >
>>
>> I would expect hdcp to use the crypto engines in some way and we don't
>> want people to feel that they should add the random clocks here, so
>> commenting them is probably the way to go.
>
> Yes HDCP uses crypto for something so those clock calls should
> stay. If the clocks were used by a HDCP device then it would be
> like the ocmem case, but it isn't.
>
>>
>> > >> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
>> > >> index 46d41e4..a934457 100644
>> > >> --- a/include/linux/qcom_scm.h
>> > >> +++ b/include/linux/qcom_scm.h
>> > >> @@ -23,10 +23,20 @@ struct qcom_scm_hdcp_req {
>> > >>       u32 val;
>> > >>  };
>> > >>
>> > >> +extern bool qcom_scm_is_available(void);
>> > >
>> > > Is this used? Looks like noise.
>> >
>> > perhaps should be split out into a separate patch..  but I am using
>> > this, and it seems like a good idea to avoid null ptr deref's of
>> > __scm.  Probably some of the scm callers should call this first..
>> > either that or we should make other scm entry points behave better if
>> > __scm is null..
>> >
>>
>> This is part of Andy's platformication, didn't he export it properly?
>> I use it as well from the remoteproc.
>
> Do we probe defer ocmem if scm isn't ready? Maybe we should name
> it qcom_scm_is_probed() and have it return -EPROBE_DEFER if it
> isn't probed and 0 if it is probed. Then drivers just call that
> function and return the error if there is one. I'd rather not
> litter scm_*() APIs with checks to see if the driver has probed
> yet. Just let those crash the system. Of course, this probably
> doesn't matter because we don't need to do any clock stuff here
> anyway.

currently, we just skip the scm calls.. which is maybe not the right
thing.  With the various kconfig options in downstream driver, I can't
say I'm really sure if there are any cases where we would skip scm
completely and do everything from kernel..

That said, I'm not a huge fan of 'crash the system' approach in
general.. it's kinda nice if the system can at least boot far enough
to tell what went wrong, especially on devices without serial ports
(or jtag, etc).

If I can assume there should always be scm, then an 'if
(!qcom_scm_is_available()) return -EPROBE_DEFER;' somewhere early on
in the probe seems like the right thing.

BR,
-R

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/4] drm/msm: add OCMEM driver
  2015-09-28 22:53     ` Rob Clark
@ 2015-09-29  1:58       ` Stephen Boyd
  0 siblings, 0 replies; 13+ messages in thread
From: Stephen Boyd @ 2015-09-29  1:58 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel, linux-arm-msm, Bjorn Andersson

On 09/28, Rob Clark wrote:
> On Mon, Sep 28, 2015 at 6:10 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 09/28, Rob Clark wrote:
> >> diff --git a/drivers/gpu/drm/msm/ocmem/ocmem.c b/drivers/gpu/drm/msm/ocmem/ocmem.c
> >> new file mode 100644
> >> index 0000000..d3cdd64
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/msm/ocmem/ocmem.c
> >> @@ -0,0 +1,396 @@
> >> +/*
> >> + * Copyright (C) 2015 Red Hat
> >> + * Author: Rob Clark <robdclark@gmail.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify it
> >> + * under the terms of the GNU General Public License version 2 as published by
> >> + * the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful, but WITHOUT
> >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> >> + * more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License along with
> >> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> >> + */
> >> +
> >> +#include <linux/kernel.h>
> >> +#include <linux/cpuset.h>
> >
> > What is this include for?
> 
> needed for qcom_scm.h, although I guess I could just add the missing
> #includes in qcom_scm.h instead..

Ok, we should fix that in scm header files. It probably needs a
forward declare of struct cpumask and it should be struct cpumask
* instead of cpumask_t *.

> 
> >> +#include <linux/qcom_scm.h>
> >> +
> >> +#include "msm_drv.h"
> >> +#include "ocmem.h"
> >> +#include "ocmem.xml.h"
> >> +
[..]
> >> +
> >> +static void update_ocmem(struct ocmem *ocmem)
> >> +{
> >> +     uint32_t region_mode_ctrl = 0x0;
> >> +     unsigned pos = 0;
> >> +     unsigned i = 0;
> >> +
> >> +     if (!qcom_scm_ocmem_lock_available()) {
> >> +             for (i = 0; i < ocmem->config->num_regions; i++) {
> >> +                     struct ocmem_region *region = &ocmem->regions[i];
> >> +                     pos = i << 2;
> >> +                     if (region->mode == THIN_MODE)
> >> +                             region_mode_ctrl |= BIT(pos);
> >> +             }
> >> +             dev_dbg(ocmem->dev, "ocmem_region_mode_control %x\n", region_mode_ctrl);
> >> +             ocmem_write(ocmem, REG_OCMEM_REGION_MODE_CTL, region_mode_ctrl);
> >> +             /* Barrier to commit the region mode */
> >> +             mb();
> >
> > msm_writel() already has a barrier, so now we have a double
> > barrier?
> 
> hmm, msm_writel() doesn't have any more barrier than writel().. so I
> kept the mb() from downstream..

Yes writel() already has a barrier. Downstream is using
writel_relaxed() instead of writel() in ocmem_write() and then
adding the barrier explicitly after ocmem_write() in the right
places.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-09-29  1:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-28 18:51 [PATCH 0/4] Add OCMEM support Rob Clark
2015-09-28 18:51 ` [PATCH 1/4] qcom-scm: add ocmem support Rob Clark
2015-09-28 20:51   ` Stephen Boyd
2015-09-28 21:08     ` Rob Clark
2015-09-28 21:59       ` Bjorn Andersson
2015-09-28 22:35         ` Stephen Boyd
2015-09-28 23:02           ` Rob Clark
2015-09-28 18:51 ` [PATCH 2/4] WIP: qcom-scm: add ocmem dump support Rob Clark
2015-09-28 18:51 ` [PATCH 3/4] drm/msm: update generated headers Rob Clark
2015-09-28 18:51 ` [PATCH 4/4] drm/msm: add OCMEM driver Rob Clark
2015-09-28 22:10   ` Stephen Boyd
2015-09-28 22:53     ` Rob Clark
2015-09-29  1:58       ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).