[Freedreno] [PATCH v3 3/4] drm/msm: re-factor devfreq code
The devfreq framework requires the drivers to provide busy time estimations.
The GPU driver relies on the hardware performance counteres for the busy time
estimations, but different hardware revisions have counters which can be
sourced from different clocks. So the busy time estimation will be target
dependent. Additionally on targets where the clocks are completely controlled
by the on chip microcontroller, fetching and setting the current GPU frequency
will be different. This patch aims to embrace these differences by re-factoring
the devfreq code a bit.
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 16 +---
drivers/gpu/drm/msm/msm_gpu.c | 49 ---
drivers/gpu/drm/msm/msm_gpu.h | 5 +++-
3 files changed, 44 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 897f3e2..043e680 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1369,12 +1369,20 @@ static struct msm_ringbuffer *a5xx_active_ring(struct
msm_gpu *gpu)
return a5xx_gpu->cur_ring;
}
-static int a5xx_gpu_busy(struct msm_gpu *gpu, uint64_t *value)
+static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu)
{
- *value = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO,
- REG_A5XX_RBBM_PERFCTR_RBBM_0_HI);
+ u64 busy_cycles;
+ unsigned long busy_time;
- return 0;
+ busy_cycles = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO,
+ REG_A5XX_RBBM_PERFCTR_RBBM_0_HI);
+
+ busy_time = (busy_cycles - gpu->devfreq.busy_cycles) /
+ (clk_get_rate(gpu->core_clk) / 100);
+
+ gpu->devfreq.busy_cycles = busy_cycles;
+
+ return busy_time;
}
static const struct adreno_gpu_funcs funcs = {
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 04f9604..6a2569a 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -36,12 +36,16 @@ static int msm_devfreq_target(struct device *dev, unsigned
long *freq,
struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
struct dev_pm_opp *opp;
- opp = dev_pm_opp_find_freq_ceil(dev, freq);
+ opp = devfreq_recommended_opp(dev, freq, flags);
+ if (IS_ERR(opp))
+ return PTR_ERR(opp);
- if (!IS_ERR(opp)) {
+ if (gpu->funcs->gpu_set_freq)
+ gpu->funcs->gpu_set_freq(gpu, (u64)*freq);
+ else
clk_set_rate(gpu->core_clk, *freq);
- dev_pm_opp_put(opp);
- }
+
+ dev_pm_opp_put(opp);
return 0;
}
@@ -50,16 +54,14 @@ static int msm_devfreq_get_dev_status(struct device *dev,
struct devfreq_dev_status *status)
{
struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
- u64 cycles;
ktime_t time;
- status->current_frequency = (unsigned long) clk_get_rate(gpu->core_clk);
- gpu->funcs->gpu_busy(gpu, );
-
- status->busy_time = (cycles - gpu->devfreq.busy_cycles) /
- (status->current_frequency / 100);
+ if (gpu->funcs->gpu_get_freq)
+ status->current_frequency = gpu->funcs->gpu_get_freq(gpu);
+ else
+ status->current_frequency = clk_get_rate(gpu->core_clk);
- gpu->devfreq.busy_cycles = cycles;
+ status->busy_time = gpu->funcs->gpu_busy(gpu);
time = ktime_get();
status->total_time = ktime_us_delta(time, gpu->devfreq.time);
@@ -72,7 +74,10 @@ static int msm_devfreq_get_cur_freq(struct device *dev,
unsigned long *freq)
{
struct msm_gpu *gpu = platform_get_drvdata(to_platform_device(dev));
- *freq = (unsigned long) clk_get_rate(gpu->core_clk);
+ if (gpu->funcs->gpu_get_freq)
+ *freq = gpu->funcs->gpu_get_freq(gpu);
+ else
+ *freq = clk_get_rate(gpu->core_clk);
return 0;
}
@@ -87,7 +92,7 @@ static int msm_devfreq_get_cur_freq(struct device *dev,
unsigned long *freq)
static void msm_devfreq_init(struct msm_gpu *gpu)
{
/* We need target support to do devfreq */
- if (!gpu->funcs->gpu_busy || !gpu->core_clk)
+ if (!gpu->funcs->gpu_busy)
return;
msm_devfreq_profile.initial_freq = gpu->fast_rate;
@@ -185,6 +190,14 @@ static int disable_axi(struct msm_gpu *gpu)
return 0;
}
+void msm_gpu_resume_devfreq(struct msm_gpu *gpu)
+{
+ gpu->devfreq.busy_cycles = 0;
+ gpu->devfreq.time = ktime_get();
+
+ devfreq_resume_device(gpu->devfreq.devfreq);
+}
+
int msm_gpu_pm_resume(struct msm_gpu *gpu)
{
int ret;
@@ -203,12 +216,7 @@ int msm_gpu_pm_resume(struct msm_gpu *gpu)
if (ret)
return ret;
- if (gpu->devfreq.devfreq) {
- gpu->devfreq.busy_cycles = 0;
- gpu->devfreq.time =
[Freedreno] [PATCH v3 2/4] drm/msm/A6xx: Add gmu_read64() register read op
Add a simple function to read 64 registers in the GMU domain
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index a08ee8f..09fd3a7 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -106,6 +106,16 @@ static inline void gmu_rmw(struct a6xx_gmu *gmu, u32 reg,
u32 mask, u32 or)
gmu_write(gmu, reg, val | or);
}
+static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32 lo, u32 hi)
+{
+ u64 val;
+
+ val = (u64) msm_readl(gmu->mmio + (lo << 2));
+ val |= ((u64) msm_readl(gmu->mmio + (hi << 2)) << 32);
+
+ return val;
+}
+
#define gmu_poll_timeout(gmu, addr, val, cond, interval, timeout) \
readl_poll_timeout((gmu)->mmio + ((addr) << 2), val, cond, \
interval, timeout)
--
1.9.1
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v3 4/4] drm/msm/A6xx: Add devfreq support for A6xx
Implement routines to estimate GPU busy time and fetching the
current frequency for the polling interval. This is required by
the devfreq framework which recommends a frequency change if needed.
The driver code then tries to set this new frequency on the GPU by
sending an Out Of Band(OOB) request to the GMU.
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 39 +++
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 ++
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 27
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 ++
4 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index f6634c0..27cc800 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -67,7 +67,7 @@ static bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF));
}
-static int a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
+static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
{
gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
@@ -84,7 +84,38 @@ static int a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
a6xx_gmu_set_oob(gmu, GMU_OOB_DCVS_SET);
a6xx_gmu_clear_oob(gmu, GMU_OOB_DCVS_SET);
- return gmu_read(gmu, REG_A6XX_GMU_DCVS_RETURN);
+ if (gmu_read(gmu, REG_A6XX_GMU_DCVS_RETURN) != 0) {
+ dev_err_ratelimited(gmu->dev, "Unable to set GPU frequency\n");
+ return;
+ }
+
+ gmu->freq = gmu->gpu_freqs[index];
+}
+
+void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct a6xx_gmu *gmu = _gpu->gmu;
+ u32 perf_index = 0;
+
+ if (freq == gmu->freq)
+ return;
+
+ for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
+ if (freq == gmu->gpu_freqs[perf_index])
+ break;
+
+ __a6xx_gmu_set_freq(gmu, perf_index);
+}
+
+unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct a6xx_gmu *gmu = _gpu->gmu;
+
+ return gmu->freq;
}
static bool a6xx_gmu_check_idle_level(struct a6xx_gmu *gmu)
@@ -630,7 +661,7 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu)
ret = a6xx_hfi_start(gmu, GMU_COLD_BOOT);
/* Set the GPU back to the highest power frequency */
- a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
+ __a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
out:
if (ret)
@@ -671,7 +702,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
ret = a6xx_hfi_start(gmu, status);
/* Set the GPU to the highest power frequency */
- a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
+ __a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
out:
/* Make sure to turn off the boot OOB request on error */
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 09fd3a7..13b064f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -77,6 +77,8 @@ struct a6xx_gmu {
unsigned long gmu_freqs[4];
u32 cx_arc_votes[4];
+ unsigned long freq;
+
struct a6xx_hfi_queue queues[2];
struct tasklet_struct hfi_tasklet;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 3429d33a..af90706 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -7,6 +7,8 @@
#include "a6xx_gpu.h"
#include "a6xx_gmu.xml.h"
+#include
+
static inline bool _a6xx_check_idle(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -682,6 +684,8 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
gpu->needs_hw_init = true;
+ msm_gpu_resume_devfreq(gpu);
+
return ret;
}
@@ -690,6 +694,8 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ devfreq_suspend_device(gpu->devfreq.devfreq);
+
/*
* Make sure the GMU is idle before continuing (because some transitions
* may use VBIF
@@ -753,6 +759,24 @@ static void a6xx_destroy(struct msm_gpu *gpu)
kfree(a6xx_gpu);
}
+static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ u64 busy_cycles;
+ unsigned long busy_time;
+
+ busy_cycles = gmu_read64(_gpu->gmu,
+
Re: [Freedreno] [PATCH 08/14] drm/msm/dpu: avoid querying for hw intf before assignment
On Tue, Aug 28, 2018 at 05:39:57PM -0700, Jeykumar Sankaran wrote:
> Resource manager assigns hw_intf blocks for the encoder only on
> modeset. If queried for hw_intf objects during init, it will be
> NULL. Since hw_intf objects are needed only during encoder enable,
s/during/after/ it looks like you need it on disable as well.
> defer the query to encoder enable which will be triggered after
> modeset.
>
Missing changelog again.
With that,
Reviewed-by: Sean Paul
> Signed-off-by: Jeykumar Sankaran
> ---
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 53
> +++---
> 1 file changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index ca0963c..6de13f4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -462,7 +462,7 @@ static void dpu_encoder_phys_vid_enable(struct
> dpu_encoder_phys *phys_enc)
> {
> struct msm_drm_private *priv;
> struct dpu_encoder_phys_vid *vid_enc;
> - struct dpu_hw_intf *intf;
> + struct dpu_rm_hw_iter iter;
> struct dpu_hw_ctl *ctl;
> u32 flush_mask = 0;
>
> @@ -474,11 +474,20 @@ static void dpu_encoder_phys_vid_enable(struct
> dpu_encoder_phys *phys_enc)
> priv = phys_enc->parent->dev->dev_private;
>
> vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> - intf = vid_enc->hw_intf;
> ctl = phys_enc->hw_ctl;
> - if (!vid_enc->hw_intf || !phys_enc->hw_ctl) {
> - DPU_ERROR("invalid hw_intf %d hw_ctl %d\n",
> - vid_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
> +
> + dpu_rm_init_hw_iter(, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
> + while (dpu_rm_get_hw(_enc->dpu_kms->rm, )) {
> + struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> +
> + if (hw_intf->idx == phys_enc->intf_idx) {
> + vid_enc->hw_intf = hw_intf;
> + break;
> + }
> + }
> +
> + if (!vid_enc->hw_intf) {
> + DPU_ERROR("hw_intf not assigned\n");
> return;
> }
>
> @@ -500,7 +509,7 @@ static void dpu_encoder_phys_vid_enable(struct
> dpu_encoder_phys *phys_enc)
> !dpu_encoder_phys_vid_is_master(phys_enc))
> goto skip_flush;
>
> - ctl->ops.get_bitmask_intf(ctl, _mask, intf->idx);
> + ctl->ops.get_bitmask_intf(ctl, _mask, vid_enc->hw_intf->idx);
> ctl->ops.update_pending_flush(ctl, flush_mask);
>
> skip_flush:
> @@ -531,22 +540,13 @@ static void dpu_encoder_phys_vid_get_hw_resources(
> struct dpu_encoder_hw_resources *hw_res,
> struct drm_connector_state *conn_state)
> {
> - struct dpu_encoder_phys_vid *vid_enc;
> -
> if (!phys_enc || !hw_res) {
> DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n",
> phys_enc != 0, hw_res != 0, conn_state != 0);
> return;
> }
>
> - vid_enc = to_dpu_encoder_phys_vid(phys_enc);
> - if (!vid_enc->hw_intf) {
> - DPU_ERROR("invalid arg(s), hw_intf\n");
> - return;
> - }
> -
> - DPU_DEBUG_VIDENC(vid_enc, "\n");
> - hw_res->intfs[vid_enc->hw_intf->idx - INTF_0] = INTF_MODE_VIDEO;
> + hw_res->intfs[phys_enc->intf_idx - INTF_0] = INTF_MODE_VIDEO;
> }
>
> static int _dpu_encoder_phys_vid_wait_for_vblank(
> @@ -781,7 +781,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
> {
> struct dpu_encoder_phys *phys_enc = NULL;
> struct dpu_encoder_phys_vid *vid_enc = NULL;
> - struct dpu_rm_hw_iter iter;
> struct dpu_encoder_irq *irq;
> int i, ret = 0;
>
> @@ -801,26 +800,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
> phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
> phys_enc->intf_idx = p->intf_idx;
>
> - /**
> - * hw_intf resource permanently assigned to this encoder
> - * Other resources allocated at atomic commit time by use case
> - */
> - dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_INTF);
> - while (dpu_rm_get_hw(>dpu_kms->rm, )) {
> - struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;
> -
> - if (hw_intf->idx == p->intf_idx) {
> - vid_enc->hw_intf = hw_intf;
> - break;
> - }
> - }
> -
> - if (!vid_enc->hw_intf) {
> - ret = -EINVAL;
> - DPU_ERROR("failed to get hw_intf\n");
> - goto fail;
> - }
> -
> DPU_DEBUG_VIDENC(vid_enc, "\n");
>
> dpu_encoder_phys_vid_init_ops(_enc->ops);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno
Re: [Freedreno] [PATCH 07/14] drm/msm/dpu: iterate for assigned hw ctl in virtual encoder
On Tue, Aug 28, 2018 at 05:39:56PM -0700, Jeykumar Sankaran wrote:
> Instead of iterating for hw ctrl per physical encoder, this
> patch moves the iterations and assignment to the virtual encoder.
>
Missing "Changes in" section as well as _why_ you're doing this in your
description.
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 22
> +++---
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 19 ---
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 ---
> 3 files changed, 19 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index b223bd2..dbf669e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1011,9 +1011,10 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> struct dpu_kms *dpu_kms;
> struct list_head *connector_list;
> struct drm_connector *conn = NULL, *conn_iter;
> - struct dpu_rm_hw_iter pp_iter;
> + struct dpu_rm_hw_iter pp_iter, ctl_iter;
> struct msm_display_topology topology;
> enum dpu_rm_topology_name topology_name;
> + struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> int i = 0, ret;
>
> if (!drm_enc) {
> @@ -1061,17 +1062,32 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) pp_iter.hw;
> }
>
> + dpu_rm_init_hw_iter(_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
> + for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> + if (!dpu_rm_get_hw(_kms->rm, _iter))
> + break;
> + hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw;
> + }
> +
> topology_name = dpu_rm_get_topology_name(topology);
> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
>
> if (phys) {
> if (!dpu_enc->hw_pp[i]) {
> - DPU_ERROR_ENC(dpu_enc,
> - "invalid pingpong block for the encoder\n");
> + DPU_ERROR_ENC(dpu_enc, "no pp block assigned"
> + "at idx: %d\n", i);
> return;
> }
> phys->hw_pp = dpu_enc->hw_pp[i];
> +
> + if (!hw_ctl[i]) {
Should you check this before you assign hw_pp?
> + DPU_ERROR_ENC(dpu_enc, "no ctl block assigned"
> + "at idx: %d\n", i);
> + return;
> + }
> + phys->hw_ctl = hw_ctl[i];
> +
> phys->connector = conn->state->connector;
> phys->topology_name = topology_name;
> if (phys->ops.mode_set)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> index c8c4612..5c89868 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
> @@ -196,9 +196,6 @@ static void dpu_encoder_phys_cmd_mode_set(
> {
> struct dpu_encoder_phys_cmd *cmd_enc =
> to_dpu_encoder_phys_cmd(phys_enc);
> - struct dpu_rm *rm = _enc->dpu_kms->rm;
> - struct dpu_rm_hw_iter iter;
> - int i, instance;
>
> if (!phys_enc || !mode || !adj_mode) {
> DPU_ERROR("invalid args\n");
> @@ -208,22 +205,6 @@ static void dpu_encoder_phys_cmd_mode_set(
> DPU_DEBUG_CMDENC(cmd_enc, "caching mode:\n");
> drm_mode_debug_printmodeline(adj_mode);
>
> - instance = phys_enc->split_role == ENC_ROLE_SLAVE ? 1 : 0;
> -
> - /* Retrieve previously allocated HW Resources. Shouldn't fail */
> - dpu_rm_init_hw_iter(, phys_enc->parent->base.id, DPU_HW_BLK_CTL);
> - for (i = 0; i <= instance; i++) {
> - if (dpu_rm_get_hw(rm, ))
> - phys_enc->hw_ctl = (struct dpu_hw_ctl *)iter.hw;
> - }
> -
> - if (IS_ERR_OR_NULL(phys_enc->hw_ctl)) {
> - DPU_ERROR_CMDENC(cmd_enc, "failed to init ctl: %ld\n",
> - PTR_ERR(phys_enc->hw_ctl));
> - phys_enc->hw_ctl = NULL;
> - return;
> - }
> -
> _dpu_encoder_phys_cmd_setup_irq_hw_idx(phys_enc);
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index ecb8c65..ca0963c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -395,9 +395,6 @@ static void dpu_encoder_phys_vid_mode_set(
> struct drm_display_mode
Re: [Freedreno] [PATCH 05/14] drm/msm/dpu: enable master-slave encoders explicitly
On Tue, Aug 28, 2018 at 05:39:54PM -0700, Jeykumar Sankaran wrote:
> Identify slave-master encoders during initialization and enable
> the encoders explicitly as the current logic has redundant and
> ambiguous loops.
Please include a "Changes in vX" section so I don't need to figure it out
myself.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 46
> ++---
> 1 file changed, 15 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 991b22c..b223bd2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -180,6 +180,7 @@ struct dpu_encoder_virt {
> unsigned int num_phys_encs;
> struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
> struct dpu_encoder_phys *cur_master;
> + struct dpu_encoder_phys *cur_slave;
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>
> bool intfs_swapped;
> @@ -1141,7 +1142,8 @@ void dpu_encoder_virt_restore(struct drm_encoder
> *drm_enc)
> static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
> {
> struct dpu_encoder_virt *dpu_enc = NULL;
> - int i, ret = 0;
> + struct dpu_encoder_phys *phys = NULL;
> + int ret = 0;
> struct drm_display_mode *cur_mode = NULL;
>
> if (!drm_enc) {
> @@ -1154,21 +1156,14 @@ static void dpu_encoder_virt_enable(struct
> drm_encoder *drm_enc)
> trace_dpu_enc_enable(DRMID(drm_enc), cur_mode->hdisplay,
>cur_mode->vdisplay);
>
> - dpu_enc->cur_master = NULL;
> - for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> - struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> + /* always enable slave encoder before master */
> + phys = dpu_enc->cur_slave;
I think this variable makes things less readable. It made sense in the loop
since
you're indented an extra level and they're obfuscated anyways, but since they're
explicit now, could you please just use dpu_enc->cur_slave/master directly?
With this nit and the added changelog in the commit addressed,
Reviewed-by: Sean Paul
> + if (phys && phys->ops.enable)
> + phys->ops.enable(phys);
>
> - if (phys && phys->ops.is_master && phys->ops.is_master(phys)) {
> - DPU_DEBUG_ENC(dpu_enc, "master is now idx %d\n", i);
> - dpu_enc->cur_master = phys;
> - break;
> - }
> - }
> -
> - if (!dpu_enc->cur_master) {
> - DPU_ERROR("virt encoder has no master! num_phys %d\n", i);
> - return;
> - }
> + phys = dpu_enc->cur_master;
> + if (phys && phys->ops.enable)
> + phys->ops.enable(phys);
>
> ret = dpu_encoder_resource_control(drm_enc, DPU_ENC_RC_EVENT_KICKOFF);
> if (ret) {
> @@ -1177,21 +1172,6 @@ static void dpu_encoder_virt_enable(struct drm_encoder
> *drm_enc)
> return;
> }
>
> - for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> - struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> -
> - if (!phys)
> - continue;
> -
> - if (phys != dpu_enc->cur_master) {
> - if (phys->ops.enable)
> - phys->ops.enable(phys);
> - }
> - }
> -
> - if (dpu_enc->cur_master->ops.enable)
> - dpu_enc->cur_master->ops.enable(dpu_enc->cur_master);
> -
> _dpu_encoder_virt_enable_helper(drm_enc);
> }
>
> @@ -2062,6 +2042,11 @@ static int dpu_encoder_virt_add_phys_encs(
> ++dpu_enc->num_phys_encs;
> }
>
> + if (params->split_role == ENC_ROLE_SLAVE)
> + dpu_enc->cur_slave = enc;
> + else
> + dpu_enc->cur_master = enc;
> +
> return 0;
> }
>
> @@ -2228,7 +2213,6 @@ int dpu_encoder_setup(struct drm_device *dev, struct
> drm_encoder *enc,
> if (ret)
> goto fail;
>
> - dpu_enc->cur_master = NULL;
> spin_lock_init(_enc->enc_spinlock);
>
> atomic_set(_enc->frame_done_timeout, 0);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 01/14] drm/msm/dpu: remove debugfs support for misr
On Tue, Aug 28, 2018 at 05:39:50PM -0700, Jeykumar Sankaran wrote:
> Strip down debugfs support for misr data read on layer mixers
> and interfaces.
Could you please start explaining not only the what, but also the why? I'm
certain there's a very good reason for this change, however you don't state it,
so I'm left to figure it out on my own. No one has ever said "your commit
message is too detailed, please make it more vague" :)
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 139
> -
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 6 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 127 ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 6 -
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 28 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 29 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 7 --
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.c | 29 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_lm.h | 7 --
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c| 3 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h| 6 -
> 11 files changed, 387 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 07c2d15..a4fab042 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -47,8 +47,6 @@
> #define LEFT_MIXER 0
> #define RIGHT_MIXER 1
>
> -#define MISR_BUFF_SIZE 256
> -
> static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> {
> struct msm_drm_private *priv;
> @@ -1272,8 +1270,6 @@ static void dpu_crtc_handle_power_event(u32 event_type,
> void *arg)
> struct drm_crtc *crtc = arg;
> struct dpu_crtc *dpu_crtc;
> struct drm_encoder *encoder;
> - struct dpu_crtc_mixer *m;
> - u32 i, misr_status;
>
> if (!crtc) {
> DPU_ERROR("invalid crtc\n");
> @@ -1294,29 +1290,8 @@ static void dpu_crtc_handle_power_event(u32
> event_type, void *arg)
>
> dpu_encoder_virt_restore(encoder);
> }
> -
> - for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> - m = _crtc->mixers[i];
> - if (!m->hw_lm || !m->hw_lm->ops.setup_misr ||
> - !dpu_crtc->misr_enable)
> - continue;
> -
> - m->hw_lm->ops.setup_misr(m->hw_lm, true,
> - dpu_crtc->misr_frame_count);
> - }
> break;
> case DPU_POWER_EVENT_PRE_DISABLE:
It looks like no one uses PRE_DISABLE now so it can be removed, ditto for
PRE_ENABLE. Since they're called at the same time anyways, lets just merge
PRE/POST_ENABLE/DISABLE into DPU_POWER_EVENT_ENABLE and DPU_POWER_EVENT_DISABLE.
In a separate patch, of course.
> - for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> - m = _crtc->mixers[i];
> - if (!m->hw_lm || !m->hw_lm->ops.collect_misr ||
> - !dpu_crtc->misr_enable)
> - continue;
> -
> - misr_status = m->hw_lm->ops.collect_misr(m->hw_lm);
> - dpu_crtc->misr_data[i] = misr_status ? misr_status :
> - dpu_crtc->misr_data[i];
> - }
> - break;
> case DPU_POWER_EVENT_POST_DISABLE:
> /**
>* Nothing to do. All the planes on the CRTC will be
> @@ -1847,113 +1822,6 @@ static int _dpu_debugfs_status_open(struct inode
> *inode, struct file *file)
> return single_open(file, _dpu_debugfs_status_show, inode->i_private);
> }
>
> -static ssize_t _dpu_crtc_misr_setup(struct file *file,
> - const char __user *user_buf, size_t count, loff_t *ppos)
> -{
> - struct dpu_crtc *dpu_crtc;
> - struct dpu_crtc_mixer *m;
> - int i = 0, rc;
> - char buf[MISR_BUFF_SIZE + 1];
> - u32 frame_count, enable;
> - size_t buff_copy;
> -
> - if (!file || !file->private_data)
> - return -EINVAL;
> -
> - dpu_crtc = file->private_data;
> - buff_copy = min_t(size_t, count, MISR_BUFF_SIZE);
> - if (copy_from_user(buf, user_buf, buff_copy)) {
> - DPU_ERROR("buffer copy failed\n");
> - return -EINVAL;
> - }
> -
> - buf[buff_copy] = 0; /* end of string */
> -
> - if (sscanf(buf, "%u %u", , _count) != 2)
> - return -EINVAL;
> -
> - rc = _dpu_crtc_power_enable(dpu_crtc, true);
> - if (rc)
> - return rc;
> -
> - mutex_lock(_crtc->crtc_lock);
> - dpu_crtc->misr_enable = enable;
> - dpu_crtc->misr_frame_count = frame_count;
> - for (i = 0; i < dpu_crtc->num_mixers; ++i) {
> -
Re: [Freedreno] [PATCH 0/3] use encoder type to identify display
On Tue, Aug 28, 2018 at 04:02:15PM -0700, Jeykumar Sankaran wrote: > DPU is broken with patch [1]. This patch cleans up few stale codes > and fixes the DPU bug by using encoder type to identify displays. > > [1] https://patchwork.kernel.org/patch/10568269/ > > Thanks. > > Jeykumar Sankaran (3): > drm/msm/dpu: remove stale display port programming > drm/msm/dpu: remove unwanted encoder type mapping > drm/msm/dpu: use encoder type to identify display type Entire series is Reviewed-by: Sean Paul I've pushed them to dpu-staging/for-next Thanks, Sean > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32 > - > 1 file changed, 9 insertions(+), 23 deletions(-) > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v16 3/5] iommu/arm-smmu: Add the device_link between masters and smmu
From: Sricharan R Finally add the device link between the master device and smmu, so that the smmu gets runtime enabled/disabled only when the master needs it. This is done from add_device callback which gets called once when the master is added to the smmu. Signed-off-by: Sricharan R Signed-off-by: Vivek Gautam Reviewed-by: Tomasz Figa Tested-by: Srinivas Kandagatla --- drivers/iommu/arm-smmu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 1bf542010be7..166c8c6da24f 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1461,6 +1461,9 @@ static int arm_smmu_add_device(struct device *dev) iommu_device_link(>iommu, dev); + device_link_add(dev, smmu->dev, + DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER); + return 0; out_cfg_free: -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v16 5/5] iommu/arm-smmu: Add support for qcom, smmu-v2 variant
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.
Signed-off-by: Vivek Gautam
Reviewed-by: Rob Herring
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
---
drivers/iommu/arm-smmu.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 166c8c6da24f..411e5ac57c64 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -119,6 +119,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -1970,6 +1971,17 @@ ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K,
GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+static const char * const qcom_smmuv2_clks[] = {
+ "bus", "iface",
+};
+
+static const struct arm_smmu_match_data qcom_smmuv2 = {
+ .version = ARM_SMMU_V2,
+ .model = QCOM_SMMUV2,
+ .clks = qcom_smmuv2_clks,
+ .num_clks = ARRAY_SIZE(qcom_smmuv2_clks),
+};
+
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = _generic_v1 },
{ .compatible = "arm,smmu-v2", .data = _generic_v2 },
@@ -1977,6 +1989,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = _mmu401 },
{ .compatible = "arm,mmu-500", .data = _mmu500 },
{ .compatible = "cavium,smmu-v2", .data = _smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = _smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v16 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2
Add bindings doc for Qcom's smmu-v2 implementation.
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
---
.../devicetree/bindings/iommu/arm,smmu.txt | 39 ++
1 file changed, 39 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce12af5..a6504b37cc21 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -17,10 +17,16 @@ conditions.
"arm,mmu-401"
"arm,mmu-500"
"cavium,smmu-v2"
+"qcom,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
+ Qcom SoCs must contain, as below, SoC-specific compatibles
+ along with "qcom,smmu-v2":
+ "qcom,msm8996-smmu-v2", "qcom,smmu-v2",
+ "qcom,sdm845-smmu-v2", "qcom,smmu-v2".
+
- reg : Base address and size of the SMMU.
- #global-interrupts : The number of global interrupts exposed by the
@@ -71,6 +77,22 @@ conditions.
or using stream matching with #iommu-cells = <2>, and
may be ignored if present in such cases.
+- clock-names:List of the names of clocks input to the device. The
+ required list depends on particular implementation and
+ is as follows:
+ - for "qcom,smmu-v2":
+- "bus": clock required for downstream bus access and
+ for the smmu ptw,
+- "iface": clock required to access smmu's registers
+ through the TCU's programming interface.
+ - unspecified for other implementations.
+
+- clocks: Specifiers for all clocks listed in the clock-names property,
+ as per generic clock bindings.
+
+- power-domains: Specifiers for power domains required to be powered on for
+ the SMMU to operate, as per generic power domain bindings.
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -137,3 +159,20 @@ conditions.
iommu-map = <0 0 0x400>;
...
};
+
+ /* Qcom's arm,smmu-v2 implementation */
+ smmu4: iommu@d0 {
+ compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
+ reg = <0xd0 0x1>;
+
+ #global-interrupts = <1>;
+ interrupts = ,
+,
+;
+ #iommu-cells = <1>;
+ power-domains = < MDSS_GDSC>;
+
+ clocks = < SMMU_MDP_AXI_CLK>,
+< SMMU_MDP_AHB_CLK>;
+ clock-names = "bus", "iface";
+ };
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v16 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
From: Sricharan R
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Also, while we enable the runtime pm add a pm sleep suspend
callback that pushes devices to low power state by turning
the clocks off in a system sleep.
Also add corresponding clock enable path in resume callback.
Signed-off-by: Sricharan R
Signed-off-by: Archit Taneja
[vivek: rework for clock and pm ops]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
---
drivers/iommu/arm-smmu.c | 77 ++--
1 file changed, 74 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index fd1b80ef9490..d900e007c3c9 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
@@ -205,6 +206,8 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int*irqs;
+ struct clk_bulk_data*clks;
+ int num_clks;
u32 cavium_id_base; /* Specific to Cavium */
@@ -1896,10 +1899,12 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ const char * const *clks;
+ int num_clks;
};
#define ARM_SMMU_MATCH_DATA(name, ver, imp)\
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
+static const struct arm_smmu_match_data name = { .version = ver, .model = imp }
ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
@@ -1918,6 +1923,23 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
+static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
+ const char * const *clks)
+{
+ int i;
+
+ if (smmu->num_clks < 1)
+ return;
+
+ smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
+ sizeof(*smmu->clks), GFP_KERNEL);
+ if (!smmu->clks)
+ return;
+
+ for (i = 0; i < smmu->num_clks; i++)
+ smmu->clks[i].id = clks[i];
+}
+
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
{
@@ -2000,6 +2022,9 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->num_clks = data->num_clks;
+
+ arm_smmu_fill_clk_data(smmu, data->clks);
parse_driver_options(smmu);
@@ -2098,6 +2123,14 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
smmu->irqs[i] = irq;
}
+ err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
+ err = clk_bulk_prepare_enable(smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2184,6 +2217,9 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);
+
return 0;
}
@@ -2192,15 +2228,50 @@ static void arm_smmu_device_shutdown(struct
platform_device *pdev)
arm_smmu_device_remove(pdev);
}
-static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
{
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
+ if (ret)
+ return ret;
arm_smmu_device_reset(smmu);
+
return 0;
}
-static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
+static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ clk_bulk_disable(smmu->num_clks, smmu->clks);
+
+ return 0;
+}
+
+static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+{
+ if
[Freedreno] [PATCH v16 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
From: Sricharan R
The smmu device probe/remove and add/remove master device callbacks
gets called when the smmu is not linked to its master, that is without
the context of the master device. So calling runtime apis in those places
separately.
Global locks are also initialized before enabling runtime pm as the
runtime_resume() calls device_reset() which does tlb_sync_global()
that ultimately requires locks to be initialized.
Signed-off-by: Sricharan R
[vivek: Cleanup pm runtime calls]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
---
drivers/iommu/arm-smmu.c | 89 +++-
1 file changed, 81 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index d900e007c3c9..1bf542010be7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -268,6 +268,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
{ 0, NULL},
};
+static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
+{
+ if (pm_runtime_enabled(smmu->dev))
+ return pm_runtime_get_sync(smmu->dev);
+
+ return 0;
+}
+
+static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
+{
+ if (pm_runtime_enabled(smmu->dev))
+ pm_runtime_put(smmu->dev);
+}
+
static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
{
return container_of(dom, struct arm_smmu_domain, domain);
@@ -913,11 +927,15 @@ static void arm_smmu_destroy_domain_context(struct
iommu_domain *domain)
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cfg *cfg = _domain->cfg;
- int irq;
+ int ret, irq;
if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
return;
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0)
+ return;
+
/*
* Disable the context bank and free the page tables before freeing
* it.
@@ -932,6 +950,8 @@ static void arm_smmu_destroy_domain_context(struct
iommu_domain *domain)
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
+
+ arm_smmu_rpm_put(smmu);
}
static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
@@ -1213,10 +1233,15 @@ static int arm_smmu_attach_dev(struct iommu_domain
*domain, struct device *dev)
return -ENODEV;
smmu = fwspec_smmu(fwspec);
+
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0)
+ return ret;
+
/* Ensure that the domain is finalised */
ret = arm_smmu_init_domain_context(domain, smmu);
if (ret < 0)
- return ret;
+ goto rpm_put;
/*
* Sanity check the domain. We don't support domains across
@@ -1226,33 +1251,50 @@ static int arm_smmu_attach_dev(struct iommu_domain
*domain, struct device *dev)
dev_err(dev,
"cannot attach to SMMU %s whilst already attached to
domain on SMMU %s\n",
dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
- return -EINVAL;
+ ret = -EINVAL;
+ goto rpm_put;
}
/* Looks ok, so add the device to the domain */
- return arm_smmu_domain_add_master(smmu_domain, fwspec);
+ ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
+
+rpm_put:
+ arm_smmu_rpm_put(smmu);
+ return ret;
}
static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+ struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
+ int ret;
if (!ops)
return -ENODEV;
- return ops->map(ops, iova, paddr, size, prot);
+ arm_smmu_rpm_get(smmu);
+ ret = ops->map(ops, iova, paddr, size, prot);
+ arm_smmu_rpm_put(smmu);
+
+ return ret;
}
static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
size_t size)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+ struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
+ size_t ret;
if (!ops)
return 0;
- return ops->unmap(ops, iova, size);
+ arm_smmu_rpm_get(smmu);
+ ret = ops->unmap(ops, iova, size);
+ arm_smmu_rpm_put(smmu);
+
+ return ret;
}
static void arm_smmu_iotlb_sync(struct iommu_domain *domain)
@@ -1407,7 +1449,13 @@ static int arm_smmu_add_device(struct device *dev)
while (i--)
cfg->smendx[i] = INVALID_SMENDX;
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0)
+ goto out_cfg_free;
+
ret =
[Freedreno] [PATCH v16 0/5] iommu/arm-smmu: Add runtime pm/sleep support
This series provides the support for turning on the arm-smmu's clocks/power domains using runtime pm. This is done using device links between smmu and client devices. The device link framework keeps the two devices in correct order for power-cycling across runtime PM or across system-wide PM. With addition of a new device link flag DL_FLAG_AUTOREMOVE_SUPPLIER [7], the device links created between arm-smmu and its clients will be automatically purged when arm-smmu driver unbinds from its device. As not all implementations support clock/power gating, we are checking for a valid 'smmu->dev's pm_domain' to conditionally enable the runtime power management for such smmu implementations that can support it. Otherwise, the clocks are turned to be always on in .probe until .remove. With conditional runtime pm now, we avoid touching dev->power.lock in fastpaths for smmu implementations that don't need to do anything useful with pm_runtime. This lets us to use the much-argued pm_runtime_get_sync/put_sync() calls in map/unmap callbacks so that the clients do not have to worry about handling any of the arm-smmu's power. This series also adds support for Qcom's arm-smmu-v2 variant that has different clocks and power requirements. Previous version of this patch series is @ [1]. Build tested the series based on 4.19-rc1. [v16] * Addressed review comments from Rob about soc specific compatibles. * Removed pm_runtime_get/put calls from arm_smmu_device_probe(), as doing a runtime_get() eventually calls arm_smmu_device_reset(). arm_smmu_device_reset() should be called only after arm_smmu_device_cfg_probe(). Enabling the clocks by default in the probe, and using pm_runtime_set_active() as suggested by Tomasz to disable the clocks when pm_runtime is enabled. [v15] * Added a list of valid values of '' in "qcom,-smmu-v2" compatible string as pointed out by Robin, and Rob in the thread [8]: * Added Srini's Tested-by. * Separated out the dt-bindings change from driver change into a new patch as suggested by new checkpatch warning. Rob, I took the liberty of removing your Reviewed-by (for your comment on '') for the new dt-bindings patch 4/5. Please feel free to review it again. Thanks! [v14] * Moved arm_smmu_device_reset() from arm_smmu_pm_resume() to arm_smmu_runtime_resume() so that the pm_resume callback calls only runtime_resume to resume the device. This should take care of restoring the state of smmu in systems in which smmu lose register state on power-domain collapse. [v13] Addressing Rafael's comments: * Added .suspend pm callback to disable the clocks in system wide suspend. * Added corresponding clock enable in .resume pm callback. * Explicitly enabling/disabling the clocks now when runtime PM is disabled. * device_link_add() doesn't depend on pm_runtime_enabled() as we can use device links across system suspend/resume too. Addressing Robin's comments: * Making device_link_add failures as non-fatal. * Removed IOMMU_OF_DECLARE() declaration as we don't need this after Rob's patch that removed all of these declarations. [v12] * Use new device link's flag introduced in [7] - DL_FLAG_AUTOREMOVE_SUPPLIER. With this devices links are automatically purged when arm-smmu driver unbinds. * Using pm_runtime_force_suspend() instead of pm_runtime_disable() to avoid following warning from arm_smmu_device_remove() [295711.537507] [ cut here ] [295711.544226] Unpreparing enabled smmu_mdp_ahb_clk [295711.549099] WARNING: CPU: 0 PID: 1 at ../drivers/clk/clk.c:697 clk_core_unprepare+0xd8/0xe0 ... [295711.674073] Call trace: [295711.679454] clk_core_unprepare+0xd8/0xe0 [295711.682059] clk_unprepare+0x28/0x40 [295711.685964] clk_bulk_unprepare+0x28/0x40 [295711.689701] arm_smmu_device_remove+0x88/0xd8 [295711.693692] arm_smmu_device_shutdown+0xc/0x18 [295711.698120] platform_drv_shutdown+0x20/0x30 [v11] * Some more cleanups for device link. We don't need an explicit delete for device link from the driver, but just set the flag DL_FLAG_AUTOREMOVE. device_link_add() API description says - "If the DL_FLAG_AUTOREMOVE is set, the link will be removed automatically when the consumer device driver unbinds." * Addressed the comments for 'smmu' in arm_smmu_map/unmap(). * Dropped the patch [6] that introduced device_link_del_dev() API. [v10] * Introduce device_link_del_dev() API to delete the link between given consumer and supplier devices. The users of device link do not need to store link pointer to delete the link later. They can straightaway use this API by passing consumer and supplier devices. * Made corresponding changes to arm-smmu driver patch handling the device links. * Dropped the patch [5] that was
Re: [Freedreno] [Patch v15 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
On Thu, Aug 30, 2018 at 3:04 PM Tomasz Figa wrote: > > On Thu, Aug 30, 2018 at 6:22 PM Vivek Gautam > wrote: > > > > On Mon, Aug 27, 2018 at 4:27 PM Vivek Gautam > > wrote: > > > > > > From: Sricharan R > > > > > > The smmu device probe/remove and add/remove master device callbacks > > > gets called when the smmu is not linked to its master, that is without > > > the context of the master device. So calling runtime apis in those places > > > separately. > > > > > > Signed-off-by: Sricharan R > > > [vivek: Cleanup pm runtime calls] > > > Signed-off-by: Vivek Gautam > > > Reviewed-by: Tomasz Figa > > > Tested-by: Srinivas Kandagatla > > > --- > > > > > > Changes since v14: > > > - none. > > > > > > drivers/iommu/arm-smmu.c | 101 > > > +++ > > > 1 file changed, 93 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > > > index a81224bc6637..23b4a60149b6 100644 > > > --- a/drivers/iommu/arm-smmu.c > > > +++ b/drivers/iommu/arm-smmu.c > > > > [snip] > > > > > @@ -2131,6 +2188,26 @@ static int arm_smmu_device_probe(struct > > > platform_device *pdev) > > > if (err) > > > return err; > > > > > > + /* > > > +* We want to avoid touching dev->power.lock in fastpaths unless > > > +* it's really going to do something useful - pm_runtime_enabled() > > > +* can serve as an ideal proxy for that decision. So, > > > conditionally > > > +* enable pm_runtime. > > > +*/ > > > + if (dev->pm_domain) > > > + pm_runtime_enable(dev); > > > + > > > + err = arm_smmu_rpm_get(smmu); > > > > We shouldn't be doing a runtime_get() yet, as this eventually calls > > arm_smmu_device_reset(). > > arm_smmu_device_reset() should be called only after > > arm_smmu_device_cfg_probe(). > > So, I plan to replace the pm_runtime_get/put() calls in probe() with > > simple clk_bulk_enable() > > to let the driver initialize smmu, and at the end of the probe we can > > disable the clocks and > > enable runtime pm over the device to let it take care of the device > > further-on. > > > > We can avoid the explicit clock disable by just calling > pm_runtime_set_active() before pm_runtime_enable(), assuming that what > probe does is symmetrical with the suspend callback, which would be > called after the latter. Sure, that sounds reasonable. Will use pm_runtime_set_active() instead of explicitly disabling the clocks.Thanks. Best regards Vivek > > Best regards, > Tomasz > ___ > iommu mailing list > io...@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [Patch v15 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
On Thu, Aug 30, 2018 at 6:22 PM Vivek Gautam wrote: > > On Mon, Aug 27, 2018 at 4:27 PM Vivek Gautam > wrote: > > > > From: Sricharan R > > > > The smmu device probe/remove and add/remove master device callbacks > > gets called when the smmu is not linked to its master, that is without > > the context of the master device. So calling runtime apis in those places > > separately. > > > > Signed-off-by: Sricharan R > > [vivek: Cleanup pm runtime calls] > > Signed-off-by: Vivek Gautam > > Reviewed-by: Tomasz Figa > > Tested-by: Srinivas Kandagatla > > --- > > > > Changes since v14: > > - none. > > > > drivers/iommu/arm-smmu.c | 101 > > +++ > > 1 file changed, 93 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > > index a81224bc6637..23b4a60149b6 100644 > > --- a/drivers/iommu/arm-smmu.c > > +++ b/drivers/iommu/arm-smmu.c > > [snip] > > > @@ -2131,6 +2188,26 @@ static int arm_smmu_device_probe(struct > > platform_device *pdev) > > if (err) > > return err; > > > > + /* > > +* We want to avoid touching dev->power.lock in fastpaths unless > > +* it's really going to do something useful - pm_runtime_enabled() > > +* can serve as an ideal proxy for that decision. So, conditionally > > +* enable pm_runtime. > > +*/ > > + if (dev->pm_domain) > > + pm_runtime_enable(dev); > > + > > + err = arm_smmu_rpm_get(smmu); > > We shouldn't be doing a runtime_get() yet, as this eventually calls > arm_smmu_device_reset(). > arm_smmu_device_reset() should be called only after > arm_smmu_device_cfg_probe(). > So, I plan to replace the pm_runtime_get/put() calls in probe() with > simple clk_bulk_enable() > to let the driver initialize smmu, and at the end of the probe we can > disable the clocks and > enable runtime pm over the device to let it take care of the device > further-on. > We can avoid the explicit clock disable by just calling pm_runtime_set_active() before pm_runtime_enable(), assuming that what probe does is symmetrical with the suspend callback, which would be called after the latter. Best regards, Tomasz ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [Patch v15 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
On Mon, Aug 27, 2018 at 4:27 PM Vivek Gautam
wrote:
>
> From: Sricharan R
>
> The smmu device probe/remove and add/remove master device callbacks
> gets called when the smmu is not linked to its master, that is without
> the context of the master device. So calling runtime apis in those places
> separately.
>
> Signed-off-by: Sricharan R
> [vivek: Cleanup pm runtime calls]
> Signed-off-by: Vivek Gautam
> Reviewed-by: Tomasz Figa
> Tested-by: Srinivas Kandagatla
> ---
>
> Changes since v14:
> - none.
>
> drivers/iommu/arm-smmu.c | 101
> +++
> 1 file changed, 93 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index a81224bc6637..23b4a60149b6 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
[snip]
> @@ -2131,6 +2188,26 @@ static int arm_smmu_device_probe(struct
> platform_device *pdev)
> if (err)
> return err;
>
> + /*
> +* We want to avoid touching dev->power.lock in fastpaths unless
> +* it's really going to do something useful - pm_runtime_enabled()
> +* can serve as an ideal proxy for that decision. So, conditionally
> +* enable pm_runtime.
> +*/
> + if (dev->pm_domain)
> + pm_runtime_enable(dev);
> +
> + err = arm_smmu_rpm_get(smmu);
We shouldn't be doing a runtime_get() yet, as this eventually calls
arm_smmu_device_reset().
arm_smmu_device_reset() should be called only after arm_smmu_device_cfg_probe().
So, I plan to replace the pm_runtime_get/put() calls in probe() with
simple clk_bulk_enable()
to let the driver initialize smmu, and at the end of the probe we can
disable the clocks and
enable runtime pm over the device to let it take care of the device further-on.
> + if (err < 0)
> + return err;
> +
> + /* Enable clocks explicitly if runtime PM is disabled */
> + if (!pm_runtime_enabled(dev)) {
> + err = clk_bulk_enable(smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> + }
> +
> err = arm_smmu_device_cfg_probe(smmu);
> if (err)
> return err;
[snip]
Best regards
Vivek
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
