Re: [PATCH v4 2/2] iio: vadc: Qualcomm SPMI PMIC voltage ADC driver
On Mon, 2014-11-17 at 23:12 +0100, Hartmut Knaack wrote: Ivan T. Ivanov schrieb am 12.11.2014 09:55: On Tue, 2014-11-11 at 23:39 +0100, Hartmut Knaack wrote: Ivan T. Ivanov schrieb am 11.11.2014 09:21: Hi Hartmut, On Mon, 2014-11-10 at 22:11 +0100, Hartmut Knaack wrote: Ivan T. Ivanov schrieb am 03.11.2014 16:24: From: Stanimir Varbanov svarba...@mm-sol.com The voltage ADC is peripheral of Qualcomm SPMI PMIC chips. It has 15 bits resolution and register space inside PMIC accessible across SPMI bus. The vadc driver registers itself through IIO interface. Reviewing again, I got the feeling that due to the complexity of adc reads (writing to register to start conversion, waiting a decent time for the conversion to complete, reading the result), it would be beneficial to use a mutex in vadc_read_raw or its depending functions. Hm, yes, but there is such a nice info_exist_lock :-) in core functions, which in practice serve the same purpose. I seem to miss that. Please point me in the right direction. I am referring to info_exist_lock mutex part of struct iio_dev. It protects all operations inkern.c, no? Good point, thanks for helping me there. I was wondering, is there a plan to improve this part of the code? I mean to remove per device lock and use something like try_module_get(), when clients are acquiring reference to iio channel? + + ret = of_property_read_u32(node, reg, res); For u16, there would be of_property_read_u16(). + if (ret 0) + return -ENODEV; Just return ret here? I am usually trying to follow these recommendations[1]. In practice driver core cares only for EPROBE_DEFER, ENODEV and ENXIO, while of_property_read_u32() can return ENODATA and EOVERFLOW, which did't not make sense for the core. Please point me in the right direction on this one, too. It is pretty common to pass error codes up, as it is also mentioned in [1]. Yes, I know that is common to just pass error codes up, but in this case it did't make too much sense, I think. Also take a look at realy_probe() and line 343. This doesn't convince me. Actually, within the probe_failed part, it just doesn't care about ENODEV and ENXIO as long as debug messages are disabled (which apart from some developers, is default for the vast majority of devices). For all other error codes, it will at least print an info or warning about what's going wrong (and that can be a lot of help for debugging). Well, if you insist... will change it. Thanks, Ivan -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/9] qcom: spm: Add Subsystem Power Manager driver
On Fri, Nov 14 2014 at 15:46 -0700, Stephen Boyd wrote:
On 10/24, Lina Iyer wrote:
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
new file mode 100644
index 000..ee2e3ca
--- /dev/null
+++ b/drivers/soc/qcom/spm.c
+#include linux/module.h
+#include linux/kernel.h
+#include linux/delay.h
Is this used?
OK. Will check and remove.
+#include linux/init.h
+#include linux/io.h
+#include linux/slab.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/of_device.h
+#include linux/err.h
+#include linux/platform_device.h
+#include linux/cpuidle.h
+#include linux/cpu_pm.h
+
+#include asm/proc-fns.h
+#include asm/suspend.h
+
+#include soc/qcom/pm.h
+#include soc/qcom/pm.h
+#include soc/qcom/scm.h
+#include soc/qcom/scm-boot.h
+
+
+#define SCM_CMD_TERMINATE_PC (0x2)
+#define SCM_FLUSH_FLAG_MASK(0x3)
+#define SCM_L2_ON (0x0)
+#define SCM_L2_OFF (0x1)
+#define MAX_PMIC_DATA 2
+#define MAX_SEQ_DATA 64
+
+#define SPM_CTL_INDEX 0x7f
+#define SPM_CTL_INDEX_SHIFT 4
+#define SPM_CTL_EN BIT(0)
Nitpick, why aren't these also tabbed out?
Ok
+
+/**
+ * spm_set_low_power_mode() - Configure SPM start address for low power mode
+ * @mode: SPM LPM mode to enter
+ */
+int qcom_spm_set_low_power_mode(enum pm_sleep_mode mode)
static?
Yeah, seem to have noticed and fixed.
+{
+ struct spm_driver_data *drv = this_cpu_ptr(cpu_spm_drv);
+ u32 start_index;
+ u32 ctl_val;
+
+ if (!drv-available)
+ return -ENXIO;
It would be nice if we didn't need this by only registering the
cpuidle device for this CPU once we've initialized the SPM
hardware.
I did explore it. It strays our cpuidle code away from the standard code
that we are trying to go towards with idle-states framework.
+
+ start_index = drv-reg_data-start_index[mode];
+
+ ctl_val = spm_register_read(drv, SPM_REG_SPM_CTL);
+ ctl_val = ~(SPM_CTL_INDEX SPM_CTL_INDEX_SHIFT);
+ ctl_val |= start_index SPM_CTL_INDEX_SHIFT;
+ ctl_val |= SPM_CTL_EN;
+ spm_register_write(drv, SPM_REG_SPM_CTL, ctl_val);
+
+ /* Ensure we have written the start address */
+ wmb();
+
+ return 0;
+}
+
+static int qcom_pm_collapse(unsigned long int unused)
+{
+ int ret;
+ u32 flag;
+ int cpu = smp_processor_id();
+
+ ret = scm_set_warm_boot_addr(cpu_resume, cpu);
+ if (ret) {
+ pr_err(Failed to set warm boot address for cpu %d\n, cpu);
Do we want this print? Won't it start spamming the log if we go
idle and we can't set the flag? Maybe we should just be silent
and return an error.
OK. removed.
I can remove it..
+ return ret;
+ }
+
+ flag = SCM_L2_ON SCM_FLUSH_FLAG_MASK;
+ scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
+
+ /**
+* Returns here only if there was a pending interrupt and we did not
+* power down as a result.
+*/
+ return 0;
+}
[...]
+
+static struct qcom_cpu_pm_ops lpm_ops = {
const?
Ok
+ .standby = qcom_cpu_standby,
+ .spc = qcom_cpu_spc,
+};
+
+struct platform_device qcom_cpuidle_device = {
+ .name = qcom_cpuidle,
+ .id= -1,
+ .dev.platform_data = lpm_ops,
+};
This can be created dynamically instead of living statically.
Done.
+
+static int spm_dev_probe(struct platform_device *pdev)
+{
[...]
+
+ match_id = of_match_node(spm_match_table, pdev-dev.of_node);
+ if (!match_id)
+ return -ENODEV;
+
+ drv-reg_data = match_id-data;
+ if (!drv-reg_data)
+ return -EINVAL;
This check seems useless. We control the .data field right above
this function so there better be some reg_data.
Removed.
+
+ /* Write the SPM sequences, first.. */
+ addr = drv-reg_base + drv-reg_data-reg_offset[SPM_REG_SEQ_ENTRY];
+ seq_data = (const u32 *)drv-reg_data-seq;
Why do we need a cast?
Compiler warns otherwise.
+ __iowrite32_copy(addr, seq_data, ARRAY_SIZE(drv-reg_data-seq)/4);
nitpick: space around that / please.
+
+ /* ..and then, the control registers.
+* On some SoC's if the control registers are written first and if the
+* CPU was held in reset, the reset signal could trigger the SPM state
+* machine, before the sequences are completely written.
+*/
+ spm_register_write(drv, SPM_REG_CFG, drv-reg_data-spm_cfg);
+ spm_register_write(drv, SPM_REG_DLY, drv-reg_data-spm_dly);
+ spm_register_write(drv, SPM_REG_PMIC_DLY, drv-reg_data-pmic_dly);
+
+ spm_register_write(drv, SPM_REG_PMIC_DATA_0,
+ drv-reg_data-pmic_data[0]);
+ spm_register_write(drv, SPM_REG_PMIC_DATA_1,
+ drv-reg_data-pmic_data[1]);
+
+ /**
+* Ensure all observers see the above register writes before the
+* cpuidle driver is allowed to use the SPM.
+
Re: [RFC/PATCH] ARM: Expose cpuid registers to userspace
Stephen, On Tue, Oct 28, 2014 at 01:19:12AM +, Stephen Boyd wrote: Exporting all the different possible configurations of CPUID registers to userspace via hwcaps is going to explode the hwcaps. Emulate userspace cpuid register accesses and export a new cpuid hwcap instead so that userspace can know to try to read the cpuid registers itself. Since there is a parallel thread with the musl guys around CPU feature detection, I thought I would reply to your patch as well (I missed it when it first posted as I was on holiday). First of all, not all the features are relevant to user space, so we need to make sure the hwcap bits explosion is actually real. If it is real (I personally doubt it), we should only expose those fields which are relevant to user space - ID_ISAR*, part of ID_PFR*. The second issue is that you don't want to expose features which the kernel does not support (e.g. Neon disabled, crypto would also not be supported; debug/PMU is another example). So masking or downgrading bit fields is necessary. Thirdly, there are reserved bits in the CPUID registers which we don't want to expose. What if you run an older kernel on a newer hardware which actually has something to those bits but the kernel doesn't support them? Fourthly, there is an auxiliary feature register (ID_AFR0) which is implementation defined. I'm not sure what user space could do with this. It needs to be read in combination with MIDR to make any sense but even though you can't tell before whether the kernel should expose such information to users. And lastly, there are big.LITTLE systems, so the kernel needs to provide a common set of features. -- Catalin -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] drm/msm/mdp5: introduce mdp5_cfg module
The hardware configuration modification from a version to another
is quite consequent. Introducing a configuration module
(mdp5_cfg) may make things more clear and easier to access when a
new hardware version comes up.
Signed-off-by: Stephane Viau sv...@codeaurora.org
---
drivers/gpu/drm/msm/Makefile| 1 +
drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 215
drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 88 +
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 211 ++-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 39 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c | 9 +-
6 files changed, 354 insertions(+), 209 deletions(-)
create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 6283dcb..51045b0 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -24,6 +24,7 @@ msm-y := \
mdp/mdp4/mdp4_irq.o \
mdp/mdp4/mdp4_kms.o \
mdp/mdp4/mdp4_plane.o \
+ mdp/mdp5/mdp5_cfg.o \
mdp/mdp5/mdp5_crtc.o \
mdp/mdp5/mdp5_encoder.o \
mdp/mdp5/mdp5_irq.o \
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
new file mode 100644
index 000..62e77d1
--- /dev/null
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c
@@ -0,0 +1,215 @@
+/*
+ * Copyright (c) 2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only 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.
+ */
+
+#include mdp5_kms.h
+#include mdp5_cfg.h
+
+struct mdp5_cfg_handler {
+ int revision;
+ struct mdp5_cfg config;
+};
+
+/* mdp5_cfg must be exposed (used in mdp5.xml.h) */
+const struct mdp5_cfg_hw *mdp5_cfg = NULL;
+
+const struct mdp5_cfg_hw msm8x74_config = {
+ .name = msm8x74,
+ .smp = {
+ .mmb_count = 22,
+ .mmb_size = 4096,
+ },
+ .ctl = {
+ .count = 5,
+ .base = { 0x00600, 0x00700, 0x00800, 0x00900, 0x00a00 },
+ },
+ .pipe_vig = {
+ .count = 3,
+ .base = { 0x01200, 0x01600, 0x01a00 },
+ },
+ .pipe_rgb = {
+ .count = 3,
+ .base = { 0x01e00, 0x02200, 0x02600 },
+ },
+ .pipe_dma = {
+ .count = 2,
+ .base = { 0x02a00, 0x02e00 },
+ },
+ .lm = {
+ .count = 5,
+ .base = { 0x03200, 0x03600, 0x03a00, 0x03e00, 0x04200 },
+ .nb_stages = 5,
+ },
+ .dspp = {
+ .count = 3,
+ .base = { 0x04600, 0x04a00, 0x04e00 },
+ },
+ .ad = {
+ .count = 2,
+ .base = { 0x13100, 0x13300 }, /* NOTE: no ad in v1.0 */
+ },
+ .intf = {
+ .count = 4,
+ .base = { 0x12500, 0x12700, 0x12900, 0x12b00 },
+ },
+ .max_clk = 2,
+};
+
+const struct mdp5_cfg_hw apq8084_config = {
+ .name = apq8084,
+ .smp = {
+ .mmb_count = 44,
+ .mmb_size = 8192,
+ .reserved_state[0] = GENMASK(7, 0), /* first 8 MMBs */
+ .reserved[CID_RGB0] = 2,
+ .reserved[CID_RGB1] = 2,
+ .reserved[CID_RGB2] = 2,
+ .reserved[CID_RGB3] = 2,
+ },
+ .ctl = {
+ .count = 5,
+ .base = { 0x00600, 0x00700, 0x00800, 0x00900, 0x00a00 },
+ },
+ .pipe_vig = {
+ .count = 4,
+ .base = { 0x01200, 0x01600, 0x01a00, 0x01e00 },
+ },
+ .pipe_rgb = {
+ .count = 4,
+ .base = { 0x02200, 0x02600, 0x02a00, 0x02e00 },
+ },
+ .pipe_dma = {
+ .count = 2,
+ .base = { 0x03200, 0x03600 },
+ },
+ .lm = {
+ .count = 6,
+ .base = { 0x03a00, 0x03e00, 0x04200, 0x04600, 0x04a00, 0x04e00
},
+ .nb_stages = 5,
+ },
+ .dspp = {
+ .count = 4,
+ .base = { 0x05200, 0x05600, 0x05a00, 0x05e00 },
+
+ },
+ .ad = {
+ .count = 3,
+ .base = { 0x13500, 0x13700, 0x13900 },
+ },
+ .intf = {
+ .count = 5,
+ .base = { 0x12500, 0x12700, 0x12900, 0x12b00, 0x12d00 },
+ },
+ .max_clk = 32000,
+};
+
+static const struct mdp5_cfg_handler cfg_handlers[] = {
+ { .revision = 0, .config = { .hw = msm8x74_config }
[PATCH 3/3] drm/msm: add multiple CRTC and overlay support
MDP5 currently support one single CRTC with its private pipe.
This change allows the configuration of multiple CRTCs with
the possibility to attach several public planes to these CRTCs.
Signed-off-by: Stephane Viau sv...@codeaurora.org
---
drivers/gpu/drm/msm/Makefile| 1 +
drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 1 +
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c| 265 +--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c | 325
drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h | 121 +++
drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 13 ++
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 48 +++-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 48 ++--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 107 +++--
9 files changed, 804 insertions(+), 125 deletions(-)
create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c
create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 51045b0..135c0e5 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -25,6 +25,7 @@ msm-y := \
mdp/mdp4/mdp4_kms.o \
mdp/mdp4/mdp4_plane.o \
mdp/mdp5/mdp5_cfg.o \
+ mdp/mdp5/mdp5_ctl.o \
mdp/mdp5/mdp5_crtc.o \
mdp/mdp5/mdp5_encoder.o \
mdp/mdp5/mdp5_irq.o \
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
index 00c8271..d0c98f9 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h
@@ -24,6 +24,7 @@
*/
extern const struct mdp5_cfg_hw *mdp5_cfg;
+#define MAX_CTL8
#define MAX_BASES 8
#define MAX_SMP_BLOCKS 44
#define MAX_CLIENTS32
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index ebe2e60..fff012a 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -1,4 +1,5 @@
/*
+ * Copyright (c) 2014 The Linux Foundation. All rights reserved.
* Copyright (C) 2013 Red Hat
* Author: Rob Clark robdcl...@gmail.com
*
@@ -22,16 +23,24 @@
#include drm_crtc_helper.h
#include drm_flip_work.h
+#define SSPP_MAX (SSPP_RGB3 + 1) /* TODO: Add SSPP_MAX in mdp5.xml.h */
+
struct mdp5_crtc {
struct drm_crtc base;
char name[8];
struct drm_plane *plane;
- struct drm_plane *planes[8];
+ struct drm_plane *planes[SSPP_MAX];
+ int plane_count;
int id;
bool enabled;
- /* which mixer/encoder we route output to: */
- int mixer;
+ /* layer mixer used for this CRTC (+ its lock): */
+#define GET_LM_ID(crtc_id) ((crtc_id == 3) ? 5 : crtc_id)
+ int lm;
+ spinlock_t lm_lock; /* protect REG_MDP5_LM_* registers */
+
+ /* CTL used for this CRTC: */
+ void *ctl;
/* if there is a pending flip, these will be non-null: */
struct drm_pending_vblank_event *event;
@@ -58,6 +67,7 @@ struct mdp5_crtc {
struct mdp_irq err;
};
#define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)
+#define is_private_plane(mdp5_crtc, drm_plane) (drm_plane ==
(mdp5_crtc)-plane)
static struct mdp5_kms *get_kms(struct drm_crtc *crtc)
{
@@ -73,26 +83,35 @@ static void request_pending(struct drm_crtc *crtc, uint32_t
pending)
mdp_irq_register(get_kms(crtc)-base, mdp5_crtc-vblank);
}
-static void crtc_flush(struct drm_crtc *crtc)
+#define mdp5_lm_get_flush(lm) mdp_ctl_flush_mask_lm(lm)
+
+static void crtc_flush(struct drm_crtc *crtc, u32 flush_mask)
{
struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
- struct mdp5_kms *mdp5_kms = get_kms(crtc);
- int id = mdp5_crtc-id;
- uint32_t i, flush = 0;
+
+ DBG(%s: flush=%08x, mdp5_crtc-name, flush_mask);
+ mdp5_ctl_commit(mdp5_crtc-ctl, flush_mask);
+}
+
+/*
+ * flush updates, to make sure hw is updated to new scanout fb,
+ * so that we can safely queue unref to current fb (ie. next
+ * vblank we know hw is done w/ previous scanout_fb).
+ */
+static void crtc_flush_all(struct drm_crtc *crtc)
+{
+ struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
+ uint32_t i, flush_mask = 0;
for (i = 0; i ARRAY_SIZE(mdp5_crtc-planes); i++) {
struct drm_plane *plane = mdp5_crtc-planes[i];
- if (plane) {
- enum mdp5_pipe pipe = mdp5_plane_pipe(plane);
- flush |= pipe2flush(pipe);
- }
+ if (plane)
+ flush_mask |= mdp5_plane_get_flush(plane);
}
- flush |= mixer2flush(mdp5_crtc-id);
- flush |= MDP5_CTL_FLUSH_CTL;
+ flush_mask |= mdp5_ctl_get_flush(mdp5_crtc-ctl);
+ flush_mask |= mdp5_lm_get_flush(mdp5_crtc-lm);
- DBG(%s: flush=%08x, mdp5_crtc-name, flush);
-
- mdp5_write(mdp5_kms,
[PATCH 0/3] Update SMP, add CFG and Overlay support
This patch set updates the SMP config for MDP5, introduces a Config module for easier platform config update and allows MDP5 driver to use multiple CRTCs as well as multiple planes (overlay). Stephane Viau (3): drm/msm/mdp5: make SMP module dynamically configurable drm/msm/mdp5: introduce mdp5_cfg module drm/msm: add multiple CRTC and overlay support drivers/gpu/drm/msm/Makefile| 2 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c | 215 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h | 89 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c| 265 +-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c | 325 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h | 121 +++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c | 13 ++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 251 - drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 97 ++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 175 +++ drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c | 247 + drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h | 22 +- 12 files changed, 1379 insertions(+), 443 deletions(-) create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.c create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_cfg.h create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.c create mode 100644 drivers/gpu/drm/msm/mdp/mdp5/mdp5_ctl.h -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] drm/msm/mdp5: make SMP module dynamically configurable
The Shared Memory Pool (SMP) has its own limitation, features and
state. Some examples are:
- the number of Memory Macro Block (MMB) and their size
- the number of lines that can be fetched
- the state of MMB currently allocated
- the computation of number of blocks required per plane
- client IDs ...
In order to avoid private data to be overwritten by other modules,
let's make these private to the SMP module.
Some of these depend on the hardware configuration, let's add them
to the mdp5_config struct.
In some hw configurations, some MMBs are statically tied to RGB
pipes and cannot be re-allocated dynamically. This change
introduces the concept of MMB static usage and makes sure that
dynamic MMB requests are dimensioned accordingly.
A note on passing a pipe pointer, instead of client IDs:
Client IDs are SMP-related information. Passing PIPE information
to SMP lets SMP module to find out which SMP client(s) are used.
This allows the SMP module to access the PIPE pointer, which can
be used for FIFO watermark configuration.
By the way, even though REG_MDP5_PIPE_REQPRIO_FIFO_WM_* registers
are part of the PIPE registers, their functionality is to reflect
the behavior of the SMP block. These registers access is now
restricted to the SMP module.
Signed-off-by: Stephane Viau sv...@codeaurora.org
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 28 +++-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h | 34 ++---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 92 ++--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.c | 242 +-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_smp.h | 22 +--
5 files changed, 265 insertions(+), 153 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index f2c15bd..0d6306d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -30,6 +30,10 @@ const struct mdp5_config *mdp5_cfg;
static const struct mdp5_config msm8x74_config = {
.name = msm8x74,
+ .smp = {
+ .mmb_count = 22,
+ .mmb_size = 4096,
+ },
.ctl = {
.count = 5,
.base = { 0x00600, 0x00700, 0x00800, 0x00900, 0x00a00 },
@@ -67,6 +71,15 @@ static const struct mdp5_config msm8x74_config = {
static const struct mdp5_config apq8084_config = {
.name = apq8084,
+ .smp = {
+ .mmb_count = 44,
+ .mmb_size = 8192,
+ .reserved_state[0] = GENMASK(7, 0), /* first 8 MMBs */
+ .reserved[CID_RGB0] = 2,
+ .reserved[CID_RGB1] = 2,
+ .reserved[CID_RGB2] = 2,
+ .reserved[CID_RGB3] = 2,
+ },
.ctl = {
.count = 5,
.base = { 0x00600, 0x00700, 0x00800, 0x00900, 0x00a00 },
@@ -222,11 +235,15 @@ static void mdp5_destroy(struct msm_kms *kms)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
struct msm_mmu *mmu = mdp5_kms-mmu;
+ void *smp = mdp5_kms-smp_priv;
if (mmu) {
mmu-funcs-detach(mmu, iommu_ports, ARRAY_SIZE(iommu_ports));
mmu-funcs-destroy(mmu);
}
+ if (smp)
+ mdp5_smp_destroy(smp);
+
kfree(mdp5_kms);
}
@@ -363,6 +380,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
struct mdp5_kms *mdp5_kms;
struct msm_kms *kms = NULL;
struct msm_mmu *mmu;
+ void *priv;
int i, ret;
mdp5_kms = kzalloc(sizeof(*mdp5_kms), GFP_KERNEL);
@@ -377,7 +395,6 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
kms = mdp5_kms-base.base;
mdp5_kms-dev = dev;
- mdp5_kms-smp_blk_cnt = config-smp_blk_cnt;
mdp5_kms-mmio = msm_ioremap(pdev, mdp_phys, MDP5);
if (IS_ERR(mdp5_kms-mmio)) {
@@ -429,6 +446,13 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
/* TODO: compute core clock rate at runtime */
clk_set_rate(mdp5_kms-src_clk, mdp5_kms-hw_cfg-max_clk);
+ priv = mdp5_smp_init(mdp5_kms-dev, mdp5_kms-hw_cfg-smp);
+ if (IS_ERR(priv)) {
+ ret = PTR_ERR(priv);
+ goto fail;
+ }
+ mdp5_kms-smp_priv = priv;
+
/* make sure things are off before attaching iommu (bootloader could
* have left things on, in which case we'll start getting faults if
* we don't disable):
@@ -489,8 +513,6 @@ static struct mdp5_platform_config *mdp5_get_config(struct
platform_device *dev)
/* TODO */
#endif
config.iommu = iommu_domain_alloc(platform_bus_type);
- /* TODO get from DT: */
- config.smp_blk_cnt = 22;
return config;
}
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
index bdbdcda..753659b 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h
@@ -23,12 +23,22 @@
#include mdp/mdp_kms.h
Re: [PATCH v9 2/9] qcom: spm: Add Subsystem Power Manager driver
On Mon, Nov 17 2014 at 14:32 -0700, Daniel Lezcano wrote:
On 10/25/2014 01:40 AM, Lina Iyer wrote:
Hi Lina,
[ ... ]
+static inline void spm_register_write(struct spm_driver_data *drv,
+ enum spm_reg reg, u32 val)
+{
+ if (drv-reg_data-reg_offset[reg])
+ writel_relaxed(val, drv-reg_base +
+ drv-reg_data-reg_offset[reg]);
Why not use writel and don't use 'wmb' below ?
+}
+
[ ... ]
Took the opportunity for optimization here, since I am writing to
essentially the same page. I dont have to barrier after every write.
+ spm_register_write(drv, SPM_REG_SPM_CTL, ctl_val);
+
+ /* Ensure we have written the start address */
+ wmb();
--
http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/ Blog
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/9] qcom: spm: Add Subsystem Power Manager driver
On Mon, Nov 17, 2014 at 1:32 PM, Daniel Lezcano
daniel.lezc...@linaro.org wrote:
On 10/25/2014 01:40 AM, Lina Iyer wrote:
Hi Lina,
[ ... ]
+static inline void spm_register_write(struct spm_driver_data *drv,
+ enum spm_reg reg, u32 val)
+{
+ if (drv-reg_data-reg_offset[reg])
+ writel_relaxed(val, drv-reg_base +
+ drv-reg_data-reg_offset[reg]);
Why not use writel and don't use 'wmb' below ?
Hi Daniel,
writel() provides ordering before the write, not after. Please have a
look at the definition.
Regards,
Bjorn
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 2/9] qcom: spm: Add Subsystem Power Manager driver
On 11/18/2014 08:56 AM, Lina Iyer wrote:
On Fri, Nov 14 2014 at 15:46 -0700, Stephen Boyd wrote:
On 10/24, Lina Iyer wrote:
+{
+struct spm_driver_data *drv = this_cpu_ptr(cpu_spm_drv);
+u32 start_index;
+u32 ctl_val;
+
+if (!drv-available)
+return -ENXIO;
It would be nice if we didn't need this by only registering the
cpuidle device for this CPU once we've initialized the SPM
hardware.
I did explore it. It strays our cpuidle code away from the standard code
that we are trying to go towards with idle-states framework.
So fix the framework?
+
+/* Write the SPM sequences, first.. */
+addr = drv-reg_base +
drv-reg_data-reg_offset[SPM_REG_SEQ_ENTRY];
+seq_data = (const u32 *)drv-reg_data-seq;
Why do we need a cast?
Compiler warns otherwise.
How?
$ cat main.c
extern int magic(const void *d);
struct m {
unsigned int data[2];
};
struct s {
const struct m *m;
};
static const struct m m = {
.data = { 0x345, 0x34},
};
static const struct s s = {
.m = m,
};
int main()
{
const unsigned int *d;
d = s.m-data;
return magic(d);
}
$ gcc -c main.c
+.of_match_table = spm_match_table,
+},
+};
+
+module_platform_driver(spm_driver);
MODULE_LICENSE()?
MODULE_ALIAS()?
MODULE_DESCRIPTION() would work?
Sure, add them all please.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
