From 9f674aaf16a468429ad9404a3d15990d2a4bd3b3 Mon Sep 17 00:00:00 2001
From: Naseer Ahmed <naseer@codeaurora.org>
Date: Tue, 11 Nov 2014 20:33:42 -0500
Subject: [PATCH] msm: mdss: refactor pipe retrieval and ref count logic
Allow only retrieval of pipes that are on particular framebuffer device
used list. This ensures that pipe is not modified from a different fb
device node and cause potential issues.
Also, modify the ref count logic to use kref and ensure pipe is freed
only while holding mutex in case there could be race condition that
pipe is allocated while being freed.
Change-Id: I626323b5df981a7ed1e03196126cd4376b5ba6a6
Signed-off-by: Adrian Salido-Moreno <adrianm@codeaurora.org>
Signed-off-by: Naseer Ahmed <naseer@codeaurora.org>
---
drivers/video/msm/mdss/mdss_mdp.h | 4 +-
drivers/video/msm/mdss/mdss_mdp_overlay.c | 59 ++++++++++++++++++-----
drivers/video/msm/mdss/mdss_mdp_pipe.c | 55 +++++++++------------
drivers/video/msm/mdss/mdss_mdp_rotator.c | 2 +-
4 files changed, 75 insertions(+), 45 deletions(-)
diff --git a/drivers/video/msm/mdss/mdss_mdp.h b/drivers/video/msm/mdss/mdss_mdp.h
index 7a97593cff48..234f0bb30204 100644
--- a/drivers/video/msm/mdss/mdss_mdp.h
+++ b/drivers/video/msm/mdss/mdss_mdp.h
@@ -20,6 +20,7 @@
#include <linux/platform_device.h>
#include <linux/notifier.h>
#include <linux/irqreturn.h>
+#include <linux/kref.h>
#include "mdss.h"
#include "mdss_mdp_hwio.h"
@@ -368,7 +369,8 @@ struct mdss_mdp_pipe {
struct mdss_mdp_shared_reg_ctrl clk_status;
struct mdss_mdp_shared_reg_ctrl sw_reset;
- atomic_t ref_cnt;
+ struct kref kref;
+
u32 play_cnt;
int pid;
bool is_handed_off;
diff --git a/drivers/video/msm/mdss/mdss_mdp_overlay.c b/drivers/video/msm/mdss/mdss_mdp_overlay.c
index 555f0cc5bf66..82882f0e747d 100644
--- a/drivers/video/msm/mdss/mdss_mdp_overlay.c
+++ b/drivers/video/msm/mdss/mdss_mdp_overlay.c
@@ -99,20 +99,36 @@ static int mdss_mdp_overlay_sd_ctrl(struct msm_fb_data_type *mfd,
return resp;
}
+static struct mdss_mdp_pipe *__overlay_find_pipe(
+ struct msm_fb_data_type *mfd, u32 ndx)
+{
+ struct mdss_overlay_private *mdp5_data = mfd_to_mdp5_data(mfd);
+ struct mdss_mdp_pipe *tmp, *pipe = NULL;
+
+ mutex_lock(&mdp5_data->list_lock);
+ list_for_each_entry(tmp, &mdp5_data->pipes_used, list) {
+ if (tmp->ndx == ndx) {
+ pipe = tmp;
+ break;
+ }
+ }
+ mutex_unlock(&mdp5_data->list_lock);
+
+ return pipe;
+}
+
static int mdss_mdp_overlay_get(struct msm_fb_data_type *mfd,
struct mdp_overlay *req)
{
struct mdss_mdp_pipe *pipe;
- struct mdss_data_type *mdata = mfd_to_mdata(mfd);
- pipe = mdss_mdp_pipe_get(mdata, req->id);
- if (IS_ERR_OR_NULL(pipe)) {
+ pipe = __overlay_find_pipe(mfd, req->id);
+ if (!pipe) {
pr_err("invalid pipe ndx=%x\n", req->id);
return pipe ? PTR_ERR(pipe) : -ENODEV;
}
*req = pipe->req_data;
- mdss_mdp_pipe_unmap(pipe);
return 0;
}
@@ -574,10 +590,17 @@ static int mdss_mdp_overlay_pipe_setup(struct msm_fb_data_type *mfd,
pipe->pid = current->tgid;
pipe->play_cnt = 0;
} else {
- pipe = mdss_mdp_pipe_get(mdp5_data->mdata, req->id);
- if (IS_ERR_OR_NULL(pipe)) {
+ pipe = __overlay_find_pipe(mfd, req->id);
+ if (!pipe) {
pr_err("invalid pipe ndx=%x\n", req->id);
- return pipe ? PTR_ERR(pipe) : -ENODEV;
+ return -ENODEV;
+ }
+
+ ret = mdss_mdp_pipe_map(pipe);
+ if (IS_ERR_VALUE(ret)) {
+ pr_err("Unable to map used pipe%d ndx=%x\n",
+ pipe->num, pipe->ndx);
+ return ret;
}
if (is_vig_needed && (pipe->type != MDSS_MDP_PIPE_TYPE_VIG)) {
@@ -1512,10 +1535,17 @@ static int mdss_mdp_overlay_queue(struct msm_fb_data_type *mfd,
u32 flags;
struct mdss_data_type *mdata = mfd_to_mdata(mfd);
- pipe = mdss_mdp_pipe_get(mdata, req->id);
- if (IS_ERR_OR_NULL(pipe)) {
+ pipe = __overlay_find_pipe(mfd, req->id);
+ if (!pipe) {
pr_err("pipe ndx=%x doesn't exist\n", req->id);
- return pipe ? PTR_ERR(pipe) : -ENODEV;
+ return -ENODEV;
+ }
+
+ ret = mdss_mdp_pipe_map(pipe);
+ if (IS_ERR_VALUE(ret)) {
+ pr_err("Unable to map used pipe%d ndx=%x\n",
+ pipe->num, pipe->ndx);
+ return ret;
}
pr_debug("ov queue pnum=%d\n", pipe->num);
@@ -1573,8 +1603,13 @@ static void mdss_mdp_overlay_force_dma_cleanup(struct mdss_data_type *mdata)
for (i = 0; i < mdata->ndma_pipes; i++) {
pipe = mdata->dma_pipes + i;
- if (atomic_read(&pipe->ref_cnt) && pipe->mfd)
- mdss_mdp_overlay_force_cleanup(pipe->mfd);
+
+ if (!mdss_mdp_pipe_map(pipe)) {
+ struct msm_fb_data_type *mfd = pipe->mfd;
+ mdss_mdp_pipe_unmap(pipe);
+ if (mfd)
+ mdss_mdp_overlay_force_cleanup(mfd);
+ }
}
}
diff --git a/drivers/video/msm/mdss/mdss_mdp_pipe.c b/drivers/video/msm/mdss/mdss_mdp_pipe.c
index 33802986bd8b..836dbfe5af42 100644
--- a/drivers/video/msm/mdss/mdss_mdp_pipe.c
+++ b/drivers/video/msm/mdss/mdss_mdp_pipe.c
@@ -38,7 +38,7 @@
static DEFINE_MUTEX(mdss_mdp_sspp_lock);
static DEFINE_MUTEX(mdss_mdp_smp_lock);
-static int mdss_mdp_pipe_free(struct mdss_mdp_pipe *pipe);
+static void mdss_mdp_pipe_free(struct kref *kref);
static int mdss_mdp_smp_mmb_set(int client_id, unsigned long *smp);
static void mdss_mdp_smp_mmb_free(unsigned long *smp, bool write);
static struct mdss_mdp_pipe *mdss_mdp_pipe_search_by_client_id(
@@ -487,21 +487,17 @@ int mdss_mdp_smp_handoff(struct mdss_data_type *mdata)
void mdss_mdp_pipe_unmap(struct mdss_mdp_pipe *pipe)
{
- int tmp;
-
- tmp = atomic_dec_return(&pipe->ref_cnt);
-
- WARN(tmp < 0, "Invalid unmap with ref_cnt=%d", tmp);
- if (tmp == 0)
- mdss_mdp_pipe_free(pipe);
+ if (kref_put_mutex(&pipe->kref, mdss_mdp_pipe_free,
+ &mdss_mdp_sspp_lock)) {
+ WARN(1, "Unexpected free pipe during unmap");
+ mutex_unlock(&mdss_mdp_sspp_lock);
+ }
}
int mdss_mdp_pipe_map(struct mdss_mdp_pipe *pipe)
{
- if (!atomic_inc_not_zero(&pipe->ref_cnt)) {
- pr_err("attempting to map unallocated pipe (%d)", pipe->num);
+ if (!kref_get_unless_zero(&pipe->kref))
return -EINVAL;
- }
return 0;
}
@@ -547,7 +543,7 @@ static struct mdss_mdp_pipe *mdss_mdp_pipe_init(struct mdss_mdp_mixer *mixer,
for (i = off; i < npipes; i++) {
pipe = pipe_pool + i;
- if (atomic_cmpxchg(&pipe->ref_cnt, 0, 1) == 0) {
+ if (atomic_read(&pipe->kref.refcount) == 0) {
pipe->mixer_left = mixer;
break;
}
@@ -558,14 +554,12 @@ static struct mdss_mdp_pipe *mdss_mdp_pipe_init(struct mdss_mdp_mixer *mixer,
pipe->priority <= left_blend_pipe->priority) {
pr_debug("priority limitation. l_pipe_prio:%d r_pipe_prio:%d\n",
left_blend_pipe->priority, pipe->priority);
- atomic_dec(&pipe->ref_cnt);
return NULL;
}
if (pipe && mdss_mdp_pipe_fetch_halt(pipe)) {
pr_err("%d failed because pipe is in bad state\n",
pipe->num);
- atomic_dec(&pipe->ref_cnt);
return NULL;
}
@@ -589,6 +583,7 @@ static struct mdss_mdp_pipe *mdss_mdp_pipe_init(struct mdss_mdp_mixer *mixer,
pr_debug("type=%x pnum=%d\n", pipe->type, pipe->num);
mutex_init(&pipe->pp_res.hist.hist_mutex);
spin_lock_init(&pipe->pp_res.hist.hist_lock);
+ kref_init(&pipe->kref);
} else if (pipe_share) {
/*
* when there is no dedicated wfd blk, DMA pipe can be
@@ -597,7 +592,7 @@ static struct mdss_mdp_pipe *mdss_mdp_pipe_init(struct mdss_mdp_mixer *mixer,
pipe = mdata->dma_pipes + mixer->num;
if (pipe->mixer_left->type != MDSS_MDP_MIXER_TYPE_WRITEBACK)
return NULL;
- atomic_inc(&pipe->ref_cnt);
+ kref_get(&pipe->kref);
pr_debug("pipe sharing for pipe=%d\n", pipe->num);
} else {
pr_err("no %d type pipes available\n", type);
@@ -620,7 +615,7 @@ struct mdss_mdp_pipe *mdss_mdp_pipe_alloc_dma(struct mdss_mdp_mixer *mixer)
} else if (pipe != &mdata->dma_pipes[mixer->num]) {
pr_err("Requested DMA pnum=%d not available\n",
mdata->dma_pipes[mixer->num].num);
- mdss_mdp_pipe_unmap(pipe);
+ kref_put(&pipe->kref, mdss_mdp_pipe_free);
pipe = NULL;
} else {
pipe->mixer_left = mixer;
@@ -707,10 +702,13 @@ struct mdss_mdp_pipe *mdss_mdp_pipe_search(struct mdss_data_type *mdata,
return NULL;
}
-static int mdss_mdp_pipe_free(struct mdss_mdp_pipe *pipe)
+static void mdss_mdp_pipe_free(struct kref *kref)
{
- pr_debug("ndx=%x pnum=%d ref_cnt=%d\n", pipe->ndx, pipe->num,
- atomic_read(&pipe->ref_cnt));
+ struct mdss_mdp_pipe *pipe;
+
+ pipe = container_of(kref, struct mdss_mdp_pipe, kref);
+
+ pr_debug("ndx=%x pnum=%d\n", pipe->ndx, pipe->num);
if (pipe->play_cnt) {
mdss_mdp_clk_ctrl(MDP_BLOCK_POWER_ON, false);
@@ -729,8 +727,6 @@ static int mdss_mdp_pipe_free(struct mdss_mdp_pipe *pipe)
pipe->mfd = NULL;
pipe->mixer_left = pipe->mixer_right = NULL;
memset(&pipe->scale, 0, sizeof(struct mdp_scale_data));
-
- return 0;
}
static bool mdss_mdp_check_pipe_in_use(struct mdss_mdp_pipe *pipe)
@@ -901,19 +897,16 @@ int mdss_mdp_pipe_fetch_halt(struct mdss_mdp_pipe *pipe)
int mdss_mdp_pipe_destroy(struct mdss_mdp_pipe *pipe)
{
- int tmp;
-
- tmp = atomic_dec_return(&pipe->ref_cnt);
-
- if (tmp != 0) {
- pr_err("unable to free pipe %d while still in use (%d)\n",
- pipe->num, tmp);
+ if (!kref_put_mutex(&pipe->kref, mdss_mdp_pipe_free,
+ &mdss_mdp_sspp_lock)) {
+ pr_err("unable to free pipe %d while still in use\n",
+ pipe->num);
return -EBUSY;
}
- mdss_mdp_pipe_free(pipe);
- return 0;
+ mutex_unlock(&mdss_mdp_sspp_lock);
+ return 0;
}
/**
@@ -972,7 +965,7 @@ int mdss_mdp_pipe_handoff(struct mdss_mdp_pipe *pipe)
pipe->is_handed_off = true;
pipe->play_cnt = 1;
- atomic_inc(&pipe->ref_cnt);
+ kref_init(&pipe->kref);
error:
return rc;
diff --git a/drivers/video/msm/mdss/mdss_mdp_rotator.c b/drivers/video/msm/mdss/mdss_mdp_rotator.c
index 60f3804a45b7..6546bf0474d6 100644
--- a/drivers/video/msm/mdss/mdss_mdp_rotator.c
+++ b/drivers/video/msm/mdss/mdss_mdp_rotator.c
@@ -644,7 +644,7 @@ static int mdss_mdp_rotator_finish(struct mdss_mdp_rotator_session *rot)
if (rot_pipe) {
struct mdss_mdp_mixer *mixer = rot_pipe->mixer_left;
- mdss_mdp_pipe_unmap(rot_pipe);
+ mdss_mdp_pipe_destroy(rot_pipe);
tmp = mdss_mdp_ctl_mixer_switch(mixer->ctl,
MDSS_MDP_WB_CTL_TYPE_BLOCK);
if (!tmp)
--
GitLab