* [PATCH 0/5] drm/msm/dpu: Clean up dpu code
@ 2018-09-19 15:44 Bruce Wang
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Bruce Wang @ 2018-09-19 15:44 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: Bruce Wang, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
seanpaul-F7+t8E8rja9g9hUCZPvPmw,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
Removes unneeded checks and unused variables. Changes some
functions that do not need return values to void.
Bruce Wang (5):
[patch 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane
[patch 2/5] drm/msm/dpu: Clean up plane atomic disable/update
[patch 3/5] drm/msm/dpu: Remove impossible checks
[patch 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
[patch 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock to
void
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 198 +++-------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 100 ++---------
2 files changed, 42 insertions(+), 256 deletions(-)
--
2.19.0.397.gdd90340f6a-goog
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2018-09-19 15:44 ` Bruce Wang
[not found] ` <20180919154415.131292-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update Bruce Wang
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Bruce Wang @ 2018-09-19 15:44 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: Bruce Wang, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
seanpaul-F7+t8E8rja9g9hUCZPvPmw,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
Removes some checks from dpu_plane.c that will never result in an error.
Subsequent variable assignments become part of the initialization wherever
possible.
Signed-off-by: Bruce Wang <bzwang@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 53 +++--------------------
1 file changed, 6 insertions(+), 47 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1ce76460d710..a8d00f57f06a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -229,19 +229,11 @@ static u64 _dpu_plane_get_qos_lut(const struct dpu_qos_lut_tbl *tbl,
static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
struct drm_framebuffer *fb)
{
- struct dpu_plane *pdpu;
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
const struct dpu_format *fmt = NULL;
u64 qos_lut;
u32 total_fl = 0, lut_usage;
- if (!plane || !fb) {
- DPU_ERROR("invalid arguments plane %d fb %d\n",
- plane != 0, fb != 0);
- return;
- }
-
- pdpu = to_dpu_plane(plane);
-
if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
DPU_ERROR("invalid arguments\n");
return;
@@ -290,17 +282,10 @@ static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
struct drm_framebuffer *fb)
{
- struct dpu_plane *pdpu;
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
const struct dpu_format *fmt = NULL;
u32 danger_lut, safe_lut;
- if (!plane || !fb) {
- DPU_ERROR("invalid arguments\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
-
if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
DPU_ERROR("invalid arguments\n");
return;
@@ -361,14 +346,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
bool enable, u32 flags)
{
- struct dpu_plane *pdpu;
-
- if (!plane) {
- DPU_ERROR("invalid arguments\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {
DPU_ERROR("invalid arguments\n");
@@ -452,16 +430,9 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
{
struct dpu_plane *pdpu;
struct dpu_vbif_set_ot_params ot_params;
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = plane->dev->dev_private;
struct dpu_kms *dpu_kms;
- if (!plane || !plane->dev || !crtc) {
- DPU_ERROR("invalid arguments plane %d crtc %d\n",
- plane != 0, crtc != 0);
- return;
- }
-
- priv = plane->dev->dev_private;
if (!priv || !priv->kms) {
DPU_ERROR("invalid KMS reference\n");
return;
@@ -496,15 +467,9 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane)
{
struct dpu_plane *pdpu;
struct dpu_vbif_set_qos_params qos_params;
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = plane->dev->dev_private;
struct dpu_kms *dpu_kms;
- if (!plane || !plane->dev) {
- DPU_ERROR("invalid arguments\n");
- return;
- }
-
- priv = plane->dev->dev_private;
if (!priv || !priv->kms) {
DPU_ERROR("invalid KMS reference\n");
return;
@@ -1737,17 +1702,11 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
struct drm_plane *plane = NULL, *master_plane = NULL;
const struct dpu_format_extended *format_list;
struct dpu_plane *pdpu;
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = dev->dev_private;
struct dpu_kms *kms;
int zpos_max = DPU_ZPOS_MAX;
int ret = -EINVAL;
- if (!dev) {
- DPU_ERROR("[%u]device is NULL\n", pipe);
- goto exit;
- }
-
- priv = dev->dev_private;
if (!priv) {
DPU_ERROR("[%u]private data is NULL\n", pipe);
goto exit;
--
2.19.0.397.gdd90340f6a-goog
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane Bruce Wang
@ 2018-09-19 15:44 ` Bruce Wang
[not found] ` <20180919154415.131292-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 3/5] drm/msm/dpu: Remove impossible checks Bruce Wang
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Bruce Wang @ 2018-09-19 15:44 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: Bruce Wang, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
seanpaul-F7+t8E8rja9g9hUCZPvPmw,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
Removes unnecessary checks from dpu_plane_atomic_disable, old_state
argument for both dpu_plane_atomic_disable and
dpu_plane_sspp_atomic_update is removed as it is no longer used.
Signed-off-by: Bruce Wang <bzwang@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +++++------------------
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index a8d00f57f06a..7f6046166626 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1157,8 +1157,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error)
pdpu->is_error = error;
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
- struct drm_plane_state *old_state)
+static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
{
uint32_t src_flags;
struct dpu_plane *pdpu = to_dpu_plane(plane);
@@ -1271,27 +1270,11 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
_dpu_plane_set_qos_remap(plane);
}
-static void _dpu_plane_atomic_disable(struct drm_plane *plane,
- struct drm_plane_state *old_state)
+static void _dpu_plane_atomic_disable(struct drm_plane *plane)
{
- struct dpu_plane *pdpu;
- struct drm_plane_state *state;
- struct dpu_plane_state *pstate;
-
- if (!plane) {
- DPU_ERROR("invalid plane\n");
- return;
- } else if (!plane->state) {
- DPU_ERROR("invalid plane state\n");
- return;
- } else if (!old_state) {
- DPU_ERROR("invalid old state\n");
- return;
- }
-
- pdpu = to_dpu_plane(plane);
- state = plane->state;
- pstate = to_dpu_plane_state(state);
+ struct dpu_plane *pdpu = to_dpu_plane(plane);
+ struct drm_plane_state *state = plane->state;
+ struct dpu_plane_state *pstate = to_dpu_plane_state(state);
trace_dpu_plane_disable(DRMID(plane), is_dpu_plane_virtual(plane),
pstate->multirect_mode);
@@ -1315,9 +1298,9 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
DPU_DEBUG_PLANE(pdpu, "\n");
if (!state->visible) {
- _dpu_plane_atomic_disable(plane, old_state);
+ _dpu_plane_atomic_disable(plane);
} else {
- dpu_plane_sspp_atomic_update(plane, old_state);
+ dpu_plane_sspp_atomic_update(plane);
}
}
--
2.19.0.397.gdd90340f6a-goog
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/5] drm/msm/dpu: Remove impossible checks
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane Bruce Wang
2018-09-19 15:44 ` [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update Bruce Wang
@ 2018-09-19 15:44 ` Bruce Wang
[not found] ` <20180919154415.131292-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void Bruce Wang
2018-09-19 15:44 ` [PATCH 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock " Bruce Wang
4 siblings, 1 reply; 12+ messages in thread
From: Bruce Wang @ 2018-09-19 15:44 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: Bruce Wang, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
seanpaul-F7+t8E8rja9g9hUCZPvPmw,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
Removes additional impossible checks in dpu_plane.c and dpu_crtc.c.
Variable assignments are moved up to be initializations where
possible. Some variables are no longer used, these are removed.
Signed-off-by: Bruce Wang <bzwang@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 121 +++-------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 +--
2 files changed, 17 insertions(+), 120 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index a8f2dd7a37c7..017d16131dac 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -55,17 +55,7 @@ static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state *cstate,
static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
{
- struct msm_drm_private *priv;
-
- if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
- DPU_ERROR("invalid crtc\n");
- return NULL;
- }
- priv = crtc->dev->dev_private;
- if (!priv || !priv->kms) {
- DPU_ERROR("invalid kms\n");
- return NULL;
- }
+ struct msm_drm_private *priv = crtc->dev->dev_private;
return to_dpu_kms(priv->kms);
}
@@ -177,28 +167,17 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
struct drm_plane *plane;
struct drm_framebuffer *fb;
struct drm_plane_state *state;
- struct dpu_crtc_state *cstate;
+ struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
struct dpu_plane_state *pstate = NULL;
struct dpu_format *format;
- struct dpu_hw_ctl *ctl;
- struct dpu_hw_mixer *lm;
- struct dpu_hw_stage_cfg *stage_cfg;
+ struct dpu_hw_ctl *ctl = mixer->lm_ctl;
+ struct dpu_hw_stage_cfg *stage_cfg = &dpu_crtc->stage_cfg;
u32 flush_mask;
uint32_t stage_idx, lm_idx;
int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
bool bg_alpha_enable = false;
- if (!dpu_crtc || !mixer) {
- DPU_ERROR("invalid dpu_crtc or mixer\n");
- return;
- }
-
- ctl = mixer->lm_ctl;
- lm = mixer->hw_lm;
- stage_cfg = &dpu_crtc->stage_cfg;
- cstate = to_dpu_crtc_state(crtc->state);
-
drm_atomic_crtc_for_each_plane(plane, crtc) {
state = plane->state;
if (!state)
@@ -217,10 +196,6 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
state->fb ? state->fb->base.id : -1);
format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
- if (!format) {
- DPU_ERROR("invalid format\n");
- return;
- }
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
@@ -261,21 +236,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
*/
static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
{
- struct dpu_crtc *dpu_crtc;
- struct dpu_crtc_state *cstate;
- struct dpu_crtc_mixer *mixer;
+ struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+ struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
+ struct dpu_crtc_mixer *mixer = cstate->mixers;
struct dpu_hw_ctl *ctl;
struct dpu_hw_mixer *lm;
-
int i;
- if (!crtc)
- return;
-
- dpu_crtc = to_dpu_crtc(crtc);
- cstate = to_dpu_crtc_state(crtc->state);
- mixer = cstate->mixers;
-
DPU_DEBUG("%s\n", dpu_crtc->name);
for (i = 0; i < cstate->num_mixers; i++) {
@@ -377,7 +344,6 @@ static void dpu_crtc_vblank_cb(void *data)
static void dpu_crtc_frame_event_work(struct kthread_work *work)
{
- struct msm_drm_private *priv;
struct dpu_crtc_frame_event *fevent;
struct drm_crtc *crtc;
struct dpu_crtc *dpu_crtc;
@@ -400,11 +366,6 @@ static void dpu_crtc_frame_event_work(struct kthread_work *work)
dpu_crtc = to_dpu_crtc(crtc);
dpu_kms = _dpu_crtc_get_kms(crtc);
- if (!dpu_kms) {
- DPU_ERROR("invalid kms handle\n");
- return;
- }
- priv = dpu_kms->dev->dev_private;
DPU_ATRACE_BEGIN("crtc_frame_event");
DRM_DEBUG_KMS("crtc%d event:%u ts:%lld\n", crtc->base.id, fevent->event,
@@ -470,11 +431,6 @@ static void dpu_crtc_frame_event_cb(void *data, u32 event)
unsigned long flags;
u32 crtc_id;
- if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
- DPU_ERROR("invalid parameters\n");
- return;
- }
-
/* Nothing to do on idle event */
if (event & DPU_ENCODER_FRAME_EVENT_IDLE)
return;
@@ -583,23 +539,12 @@ static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
- struct dpu_crtc *dpu_crtc;
- struct dpu_crtc_state *cstate;
- struct drm_display_mode *adj_mode;
- u32 crtc_split_width;
+ struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+ struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
+ struct drm_display_mode *adj_mode = &state->adjusted_mode;
+ u32 crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode);
int i;
- if (!crtc || !state) {
- DPU_ERROR("invalid args\n");
- return;
- }
-
- dpu_crtc = to_dpu_crtc(crtc);
- cstate = to_dpu_crtc_state(state);
-
- adj_mode = &state->adjusted_mode;
- crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode);
-
for (i = 0; i < cstate->num_mixers; i++) {
struct drm_rect *r = &cstate->lm_bounds[i];
r->x1 = crtc_split_width * i;
@@ -819,29 +764,12 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc)
void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
{
struct drm_encoder *encoder;
- struct drm_device *dev;
- struct dpu_crtc *dpu_crtc;
- struct msm_drm_private *priv;
- struct dpu_kms *dpu_kms;
- struct dpu_crtc_state *cstate;
+ struct drm_device *dev = crtc->dev;
+ struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+ struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
+ struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
int ret;
- if (!crtc) {
- DPU_ERROR("invalid argument\n");
- return;
- }
- dev = crtc->dev;
- dpu_crtc = to_dpu_crtc(crtc);
- dpu_kms = _dpu_crtc_get_kms(crtc);
-
- if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev_private) {
- DPU_ERROR("invalid argument\n");
- return;
- }
-
- priv = dpu_kms->dev->dev_private;
- cstate = to_dpu_crtc_state(crtc->state);
-
/*
* If no mixers has been allocated in dpu_crtc_atomic_check(),
* it means we are trying to start a CRTC whose state is disabled:
@@ -969,24 +897,9 @@ static int _dpu_crtc_vblank_enable_no_lock(
*/
static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
{
- struct dpu_crtc *dpu_crtc;
- struct msm_drm_private *priv;
- struct dpu_kms *dpu_kms;
+ struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
int ret = 0;
- if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
- DPU_ERROR("invalid crtc\n");
- return;
- }
- dpu_crtc = to_dpu_crtc(crtc);
- priv = crtc->dev->dev_private;
-
- if (!priv->kms) {
- DPU_ERROR("invalid crtc kms\n");
- return;
- }
- dpu_kms = to_dpu_kms(priv->kms);
-
DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
mutex_lock(&dpu_crtc->crtc_lock);
@@ -1673,8 +1586,6 @@ static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc)
dpu_crtc = to_dpu_crtc(crtc);
dpu_kms = _dpu_crtc_get_kms(crtc);
- if (!dpu_kms)
- return -EINVAL;
dpu_crtc->debugfs_root = debugfs_create_dir(dpu_crtc->name,
crtc->dev->primary->debugfs_root);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 7f6046166626..0a223ac94cfb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -123,13 +123,8 @@ struct dpu_plane {
static struct dpu_kms *_dpu_plane_get_kms(struct drm_plane *plane)
{
- struct msm_drm_private *priv;
+ struct msm_drm_private *priv = plane->dev->dev_private;
- if (!plane || !plane->dev)
- return NULL;
- priv = plane->dev->dev_private;
- if (!priv)
- return NULL;
return to_dpu_kms(priv->kms);
}
@@ -514,10 +509,6 @@ static int _dpu_plane_get_aspace(
}
kms = _dpu_plane_get_kms(&pdpu->base);
- if (!kms) {
- DPU_ERROR("invalid kms\n");
- return -EINVAL;
- }
*aspace = kms->base.aspace;
@@ -1690,11 +1681,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
int zpos_max = DPU_ZPOS_MAX;
int ret = -EINVAL;
- if (!priv) {
- DPU_ERROR("[%u]private data is NULL\n", pipe);
- goto exit;
- }
-
if (!priv->kms) {
DPU_ERROR("[%u]invalid KMS reference\n", pipe);
goto exit;
--
2.19.0.397.gdd90340f6a-goog
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (2 preceding siblings ...)
2018-09-19 15:44 ` [PATCH 3/5] drm/msm/dpu: Remove impossible checks Bruce Wang
@ 2018-09-19 15:44 ` Bruce Wang
[not found] ` <20180919154415.131292-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock " Bruce Wang
4 siblings, 1 reply; 12+ messages in thread
From: Bruce Wang @ 2018-09-19 15:44 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: Bruce Wang, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
seanpaul-F7+t8E8rja9g9hUCZPvPmw,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
All checks for _dpu_crtc_power_enable are not true, so the function
can never return an error code. All calls of the function have also
been changed so that they don't expect a return value.
Signed-off-by: Bruce Wang <bzwang@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 ++++--------------------
1 file changed, 5 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 017d16131dac..f0b52281c46f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -60,37 +60,16 @@ static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
return to_dpu_kms(priv->kms);
}
-static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
+static inline void _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
{
- struct drm_crtc *crtc;
- struct msm_drm_private *priv;
- struct dpu_kms *dpu_kms;
-
- if (!dpu_crtc) {
- DPU_ERROR("invalid dpu crtc\n");
- return -EINVAL;
- }
-
- crtc = &dpu_crtc->base;
- if (!crtc->dev || !crtc->dev->dev_private) {
- DPU_ERROR("invalid drm device\n");
- return -EINVAL;
- }
-
- priv = crtc->dev->dev_private;
- if (!priv->kms) {
- DPU_ERROR("invalid kms\n");
- return -EINVAL;
- }
-
- dpu_kms = to_dpu_kms(priv->kms);
+ struct drm_crtc *crtc = &dpu_crtc->base;
+ struct msm_drm_private *priv = crtc->dev->dev_private;
+ struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
if (enable)
pm_runtime_get_sync(&dpu_kms->pdev->dev);
else
pm_runtime_put_sync(&dpu_kms->pdev->dev);
-
- return 0;
}
static void dpu_crtc_destroy(struct drm_crtc *crtc)
@@ -849,14 +828,10 @@ static int _dpu_crtc_vblank_enable_no_lock(
dev = crtc->dev;
if (enable) {
- int ret;
-
/* drop lock since power crtc cb may try to re-acquire lock */
mutex_unlock(&dpu_crtc->crtc_lock);
- ret = _dpu_crtc_power_enable(dpu_crtc, true);
+ _dpu_crtc_power_enable(dpu_crtc, true);
mutex_lock(&dpu_crtc->crtc_lock);
- if (ret)
- return ret;
list_for_each_entry(enc, &dev->mode_config.encoder_list, head) {
if (enc->crtc != crtc)
--
2.19.0.397.gdd90340f6a-goog
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock to void
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (3 preceding siblings ...)
2018-09-19 15:44 ` [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void Bruce Wang
@ 2018-09-19 15:44 ` Bruce Wang
[not found] ` <20180919154415.131292-6-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
4 siblings, 1 reply; 12+ messages in thread
From: Bruce Wang @ 2018-09-19 15:44 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: Bruce Wang, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
seanpaul-F7+t8E8rja9g9hUCZPvPmw,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
Removes redundant tests for _dpu_crtc_vblank_enable_no_lock.
Function return type is now void and all function calls have
been changed accordingly.
Signed-off-by: Bruce Wang <bzwang@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 42 ++++--------------------
1 file changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f0b52281c46f..8ce24a85e70b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -809,24 +809,14 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
* _dpu_crtc_vblank_enable_no_lock - update power resource and vblank request
* @dpu_crtc: Pointer to dpu crtc structure
* @enable: Whether to enable/disable vblanks
- *
- * @Return: error code
*/
-static int _dpu_crtc_vblank_enable_no_lock(
+static void _dpu_crtc_vblank_enable_no_lock(
struct dpu_crtc *dpu_crtc, bool enable)
{
- struct drm_device *dev;
- struct drm_crtc *crtc;
+ struct drm_crtc *crtc = &dpu_crtc->base;
+ struct drm_device *dev = crtc->dev;
struct drm_encoder *enc;
- if (!dpu_crtc) {
- DPU_ERROR("invalid crtc\n");
- return -EINVAL;
- }
-
- crtc = &dpu_crtc->base;
- dev = crtc->dev;
-
if (enable) {
/* drop lock since power crtc cb may try to re-acquire lock */
mutex_unlock(&dpu_crtc->crtc_lock);
@@ -861,8 +851,6 @@ static int _dpu_crtc_vblank_enable_no_lock(
_dpu_crtc_power_enable(dpu_crtc, false);
mutex_lock(&dpu_crtc->crtc_lock);
}
-
- return 0;
}
/**
@@ -873,7 +861,6 @@ static int _dpu_crtc_vblank_enable_no_lock(
static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
{
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
- int ret = 0;
DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
@@ -888,10 +875,7 @@ static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n",
crtc->base.id, enable);
else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
}
dpu_crtc->suspend = enable;
@@ -1000,7 +984,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
struct drm_display_mode *mode;
struct drm_encoder *encoder;
struct msm_drm_private *priv;
- int ret;
unsigned long flags;
if (!crtc || !crtc->dev || !crtc->dev->dev_private || !crtc->state) {
@@ -1031,10 +1014,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
if (dpu_crtc->enabled && !dpu_crtc->suspend &&
dpu_crtc->vblank_requested) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
}
dpu_crtc->enabled = false;
@@ -1080,7 +1060,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc;
struct drm_encoder *encoder;
struct msm_drm_private *priv;
- int ret;
if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
DPU_ERROR("invalid crtc\n");
@@ -1102,10 +1081,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
if (!dpu_crtc->enabled && !dpu_crtc->suspend &&
dpu_crtc->vblank_requested) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
}
dpu_crtc->enabled = true;
@@ -1360,7 +1336,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
{
struct dpu_crtc *dpu_crtc;
- int ret;
if (!crtc) {
DPU_ERROR("invalid crtc\n");
@@ -1371,10 +1346,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
mutex_lock(&dpu_crtc->crtc_lock);
trace_dpu_crtc_vblank(DRMID(&dpu_crtc->base), en, dpu_crtc);
if (dpu_crtc->enabled && !dpu_crtc->suspend) {
- ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
- if (ret)
- DPU_ERROR("%s vblank enable failed: %d\n",
- dpu_crtc->name, ret);
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
}
dpu_crtc->vblank_requested = en;
mutex_unlock(&dpu_crtc->crtc_lock);
--
2.19.0.397.gdd90340f6a-goog
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane
[not found] ` <20180919154415.131292-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2018-09-19 19:01 ` Sean Paul
0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2018-09-19 19:01 UTC (permalink / raw)
To: Bruce Wang
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Wed, Sep 19, 2018 at 11:44:11AM -0400, Bruce Wang wrote:
> Removes some checks from dpu_plane.c that will never result in an error.
> Subsequent variable assignments become part of the initialization wherever
> possible.
>
> Signed-off-by: Bruce Wang <bzwang@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 53 +++--------------------
> 1 file changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 1ce76460d710..a8d00f57f06a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -229,19 +229,11 @@ static u64 _dpu_plane_get_qos_lut(const struct dpu_qos_lut_tbl *tbl,
> static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
> struct drm_framebuffer *fb)
> {
> - struct dpu_plane *pdpu;
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> const struct dpu_format *fmt = NULL;
> u64 qos_lut;
> u32 total_fl = 0, lut_usage;
>
> - if (!plane || !fb) {
> - DPU_ERROR("invalid arguments plane %d fb %d\n",
> - plane != 0, fb != 0);
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> -
> if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
As we discussed, it looks like we can remove all of these too \o/
> DPU_ERROR("invalid arguments\n");
> return;
> @@ -290,17 +282,10 @@ static void _dpu_plane_set_qos_lut(struct drm_plane *plane,
> static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
> struct drm_framebuffer *fb)
> {
> - struct dpu_plane *pdpu;
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> const struct dpu_format *fmt = NULL;
> u32 danger_lut, safe_lut;
>
> - if (!plane || !fb) {
> - DPU_ERROR("invalid arguments\n");
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> -
> if (!pdpu->pipe_hw || !pdpu->pipe_sblk || !pdpu->catalog) {
and here
> DPU_ERROR("invalid arguments\n");
> return;
> @@ -361,14 +346,7 @@ static void _dpu_plane_set_danger_lut(struct drm_plane *plane,
> static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
> bool enable, u32 flags)
> {
> - struct dpu_plane *pdpu;
> -
> - if (!plane) {
> - DPU_ERROR("invalid arguments\n");
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
>
> if (!pdpu->pipe_hw || !pdpu->pipe_sblk) {
and here
> DPU_ERROR("invalid arguments\n");
> @@ -452,16 +430,9 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
> {
> struct dpu_plane *pdpu;
> struct dpu_vbif_set_ot_params ot_params;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
> struct dpu_kms *dpu_kms;
>
> - if (!plane || !plane->dev || !crtc) {
> - DPU_ERROR("invalid arguments plane %d crtc %d\n",
> - plane != 0, crtc != 0);
> - return;
> - }
> -
> - priv = plane->dev->dev_private;
> if (!priv || !priv->kms) {
These are also guaranteed to be non-NULL (dev_private checked in msm_drv and kms
is checked in dpu_kms.c (or maybe msm_kms.c, but either way)).
> DPU_ERROR("invalid KMS reference\n");
> return;
> @@ -496,15 +467,9 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane)
> {
> struct dpu_plane *pdpu;
> struct dpu_vbif_set_qos_params qos_params;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
> struct dpu_kms *dpu_kms;
>
> - if (!plane || !plane->dev) {
> - DPU_ERROR("invalid arguments\n");
> - return;
> - }
> -
> - priv = plane->dev->dev_private;
> if (!priv || !priv->kms) {
> DPU_ERROR("invalid KMS reference\n");
> return;
> @@ -1737,17 +1702,11 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
> struct drm_plane *plane = NULL, *master_plane = NULL;
> const struct dpu_format_extended *format_list;
> struct dpu_plane *pdpu;
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = dev->dev_private;
> struct dpu_kms *kms;
> int zpos_max = DPU_ZPOS_MAX;
> int ret = -EINVAL;
>
> - if (!dev) {
> - DPU_ERROR("[%u]device is NULL\n", pipe);
> - goto exit;
> - }
> -
> - priv = dev->dev_private;
> if (!priv) {
So let's do another pass of all dev_private checks along with kms, pipe_hw/sblk/etc.
Sean
> DPU_ERROR("[%u]private data is NULL\n", pipe);
> goto exit;
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update
[not found] ` <20180919154415.131292-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2018-09-19 19:02 ` Sean Paul
0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2018-09-19 19:02 UTC (permalink / raw)
To: Bruce Wang
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Wed, Sep 19, 2018 at 11:44:12AM -0400, Bruce Wang wrote:
> Removes unnecessary checks from dpu_plane_atomic_disable, old_state
> argument for both dpu_plane_atomic_disable and
> dpu_plane_sspp_atomic_update is removed as it is no longer used.
>
> Signed-off-by: Bruce Wang <bzwang@chromium.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 31 +++++------------------
> 1 file changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index a8d00f57f06a..7f6046166626 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1157,8 +1157,7 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error)
> pdpu->is_error = error;
> }
>
> -static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> - struct drm_plane_state *old_state)
> +static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
> {
> uint32_t src_flags;
> struct dpu_plane *pdpu = to_dpu_plane(plane);
> @@ -1271,27 +1270,11 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> _dpu_plane_set_qos_remap(plane);
> }
>
> -static void _dpu_plane_atomic_disable(struct drm_plane *plane,
> - struct drm_plane_state *old_state)
> +static void _dpu_plane_atomic_disable(struct drm_plane *plane)
> {
> - struct dpu_plane *pdpu;
> - struct drm_plane_state *state;
> - struct dpu_plane_state *pstate;
> -
> - if (!plane) {
> - DPU_ERROR("invalid plane\n");
> - return;
> - } else if (!plane->state) {
> - DPU_ERROR("invalid plane state\n");
> - return;
> - } else if (!old_state) {
> - DPU_ERROR("invalid old state\n");
> - return;
> - }
> -
> - pdpu = to_dpu_plane(plane);
> - state = plane->state;
> - pstate = to_dpu_plane_state(state);
> + struct dpu_plane *pdpu = to_dpu_plane(plane);
> + struct drm_plane_state *state = plane->state;
> + struct dpu_plane_state *pstate = to_dpu_plane_state(state);
>
> trace_dpu_plane_disable(DRMID(plane), is_dpu_plane_virtual(plane),
> pstate->multirect_mode);
> @@ -1315,9 +1298,9 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
> DPU_DEBUG_PLANE(pdpu, "\n");
>
> if (!state->visible) {
> - _dpu_plane_atomic_disable(plane, old_state);
> + _dpu_plane_atomic_disable(plane);
> } else {
> - dpu_plane_sspp_atomic_update(plane, old_state);
> + dpu_plane_sspp_atomic_update(plane);
> }
> }
>
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] drm/msm/dpu: Remove impossible checks
[not found] ` <20180919154415.131292-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2018-09-19 19:10 ` Sean Paul
0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2018-09-19 19:10 UTC (permalink / raw)
To: Bruce Wang
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Wed, Sep 19, 2018 at 11:44:13AM -0400, Bruce Wang wrote:
> Removes additional impossible checks in dpu_plane.c and dpu_crtc.c.
> Variable assignments are moved up to be initializations where
> possible. Some variables are no longer used, these are removed.
>
> Signed-off-by: Bruce Wang <bzwang@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 121 +++-------------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 +--
The dpu_plane changes should probably get rolled into the dpu_plane removal
patch, then rename the subject of this patch "Remove impossible checks from
dpu_crtc".
> 2 files changed, 17 insertions(+), 120 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index a8f2dd7a37c7..017d16131dac 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -55,17 +55,7 @@ static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state *cstate,
>
> static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> {
> - struct msm_drm_private *priv;
> -
> - if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
> - DPU_ERROR("invalid crtc\n");
> - return NULL;
> - }
> - priv = crtc->dev->dev_private;
> - if (!priv || !priv->kms) {
> - DPU_ERROR("invalid kms\n");
> - return NULL;
> - }
> + struct msm_drm_private *priv = crtc->dev->dev_private;
>
> return to_dpu_kms(priv->kms);
Might as well just slap the whole thing in the arg and save a local:
return to_dpu_kms(crtc->dev->dev_private->kms);
> }
> @@ -177,28 +167,17 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> struct drm_plane *plane;
> struct drm_framebuffer *fb;
> struct drm_plane_state *state;
> - struct dpu_crtc_state *cstate;
> + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> struct dpu_plane_state *pstate = NULL;
> struct dpu_format *format;
> - struct dpu_hw_ctl *ctl;
> - struct dpu_hw_mixer *lm;
> - struct dpu_hw_stage_cfg *stage_cfg;
> + struct dpu_hw_ctl *ctl = mixer->lm_ctl;
> + struct dpu_hw_stage_cfg *stage_cfg = &dpu_crtc->stage_cfg;
>
> u32 flush_mask;
> uint32_t stage_idx, lm_idx;
> int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
> bool bg_alpha_enable = false;
>
> - if (!dpu_crtc || !mixer) {
> - DPU_ERROR("invalid dpu_crtc or mixer\n");
> - return;
> - }
> -
> - ctl = mixer->lm_ctl;
> - lm = mixer->hw_lm;
> - stage_cfg = &dpu_crtc->stage_cfg;
> - cstate = to_dpu_crtc_state(crtc->state);
> -
> drm_atomic_crtc_for_each_plane(plane, crtc) {
> state = plane->state;
> if (!state)
> @@ -217,10 +196,6 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> state->fb ? state->fb->base.id : -1);
>
> format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
> - if (!format) {
> - DPU_ERROR("invalid format\n");
> - return;
> - }
>
> if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
> bg_alpha_enable = true;
> @@ -261,21 +236,13 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
> */
> static void _dpu_crtc_blend_setup(struct drm_crtc *crtc)
> {
> - struct dpu_crtc *dpu_crtc;
> - struct dpu_crtc_state *cstate;
> - struct dpu_crtc_mixer *mixer;
> + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> + struct dpu_crtc_mixer *mixer = cstate->mixers;
> struct dpu_hw_ctl *ctl;
> struct dpu_hw_mixer *lm;
> -
> int i;
>
> - if (!crtc)
> - return;
> -
> - dpu_crtc = to_dpu_crtc(crtc);
> - cstate = to_dpu_crtc_state(crtc->state);
> - mixer = cstate->mixers;
> -
> DPU_DEBUG("%s\n", dpu_crtc->name);
>
> for (i = 0; i < cstate->num_mixers; i++) {
> @@ -377,7 +344,6 @@ static void dpu_crtc_vblank_cb(void *data)
>
> static void dpu_crtc_frame_event_work(struct kthread_work *work)
> {
> - struct msm_drm_private *priv;
> struct dpu_crtc_frame_event *fevent;
> struct drm_crtc *crtc;
> struct dpu_crtc *dpu_crtc;
> @@ -400,11 +366,6 @@ static void dpu_crtc_frame_event_work(struct kthread_work *work)
This is collapsed, but the if (!work) condition is also bogus.
The fevent null checks are also suspicious. Checking fevent->crtc->state
is downright dangerous since it's unlocked access. Fortunately it doesn't
look like the function is using state, so let's remove that check. Given that
you can remove a bunch more, most of these assignments can be folded up top.
> dpu_crtc = to_dpu_crtc(crtc);
>
> dpu_kms = _dpu_crtc_get_kms(crtc);
> - if (!dpu_kms) {
> - DPU_ERROR("invalid kms handle\n");
> - return;
> - }
> - priv = dpu_kms->dev->dev_private;
> DPU_ATRACE_BEGIN("crtc_frame_event");
>
> DRM_DEBUG_KMS("crtc%d event:%u ts:%lld\n", crtc->base.id, fevent->event,
> @@ -470,11 +431,6 @@ static void dpu_crtc_frame_event_cb(void *data, u32 event)
> unsigned long flags;
> u32 crtc_id;
>
> - if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
> - DPU_ERROR("invalid parameters\n");
> - return;
> - }
> -
> /* Nothing to do on idle event */
> if (event & DPU_ENCODER_FRAME_EVENT_IDLE)
> return;
> @@ -583,23 +539,12 @@ static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
> static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
> struct drm_crtc_state *state)
> {
> - struct dpu_crtc *dpu_crtc;
> - struct dpu_crtc_state *cstate;
> - struct drm_display_mode *adj_mode;
> - u32 crtc_split_width;
> + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> + struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
> + struct drm_display_mode *adj_mode = &state->adjusted_mode;
> + u32 crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode);
> int i;
>
> - if (!crtc || !state) {
> - DPU_ERROR("invalid args\n");
> - return;
> - }
> -
> - dpu_crtc = to_dpu_crtc(crtc);
> - cstate = to_dpu_crtc_state(state);
> -
> - adj_mode = &state->adjusted_mode;
> - crtc_split_width = _dpu_crtc_get_mixer_width(cstate, adj_mode);
> -
> for (i = 0; i < cstate->num_mixers; i++) {
> struct drm_rect *r = &cstate->lm_bounds[i];
> r->x1 = crtc_split_width * i;
> @@ -819,29 +764,12 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc)
> void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> {
> struct drm_encoder *encoder;
> - struct drm_device *dev;
> - struct dpu_crtc *dpu_crtc;
> - struct msm_drm_private *priv;
> - struct dpu_kms *dpu_kms;
> - struct dpu_crtc_state *cstate;
> + struct drm_device *dev = crtc->dev;
> + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> + struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> + struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> int ret;
>
> - if (!crtc) {
> - DPU_ERROR("invalid argument\n");
> - return;
> - }
> - dev = crtc->dev;
> - dpu_crtc = to_dpu_crtc(crtc);
> - dpu_kms = _dpu_crtc_get_kms(crtc);
> -
> - if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev_private) {
> - DPU_ERROR("invalid argument\n");
> - return;
> - }
> -
> - priv = dpu_kms->dev->dev_private;
> - cstate = to_dpu_crtc_state(crtc->state);
> -
> /*
> * If no mixers has been allocated in dpu_crtc_atomic_check(),
> * it means we are trying to start a CRTC whose state is disabled:
> @@ -969,24 +897,9 @@ static int _dpu_crtc_vblank_enable_no_lock(
> */
> static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
> {
> - struct dpu_crtc *dpu_crtc;
> - struct msm_drm_private *priv;
> - struct dpu_kms *dpu_kms;
> + struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> int ret = 0;
>
> - if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
> - DPU_ERROR("invalid crtc\n");
> - return;
> - }
> - dpu_crtc = to_dpu_crtc(crtc);
> - priv = crtc->dev->dev_private;
> -
> - if (!priv->kms) {
> - DPU_ERROR("invalid crtc kms\n");
> - return;
> - }
> - dpu_kms = to_dpu_kms(priv->kms);
> -
> DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
>
> mutex_lock(&dpu_crtc->crtc_lock);
> @@ -1673,8 +1586,6 @@ static int _dpu_crtc_init_debugfs(struct drm_crtc *crtc)
> dpu_crtc = to_dpu_crtc(crtc);
>
> dpu_kms = _dpu_crtc_get_kms(crtc);
> - if (!dpu_kms)
> - return -EINVAL;
>
> dpu_crtc->debugfs_root = debugfs_create_dir(dpu_crtc->name,
> crtc->dev->primary->debugfs_root);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 7f6046166626..0a223ac94cfb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -123,13 +123,8 @@ struct dpu_plane {
>
> static struct dpu_kms *_dpu_plane_get_kms(struct drm_plane *plane)
> {
> - struct msm_drm_private *priv;
> + struct msm_drm_private *priv = plane->dev->dev_private;
>
> - if (!plane || !plane->dev)
> - return NULL;
> - priv = plane->dev->dev_private;
> - if (!priv)
> - return NULL;
> return to_dpu_kms(priv->kms);
> }
>
> @@ -514,10 +509,6 @@ static int _dpu_plane_get_aspace(
> }
>
> kms = _dpu_plane_get_kms(&pdpu->base);
> - if (!kms) {
> - DPU_ERROR("invalid kms\n");
> - return -EINVAL;
> - }
>
> *aspace = kms->base.aspace;
>
> @@ -1690,11 +1681,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
> int zpos_max = DPU_ZPOS_MAX;
> int ret = -EINVAL;
>
> - if (!priv) {
> - DPU_ERROR("[%u]private data is NULL\n", pipe);
> - goto exit;
> - }
> -
> if (!priv->kms) {
This can go too
> DPU_ERROR("[%u]invalid KMS reference\n", pipe);
> goto exit;
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
[not found] ` <20180919154415.131292-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2018-09-19 19:13 ` Sean Paul
2018-09-19 19:14 ` Sean Paul
0 siblings, 1 reply; 12+ messages in thread
From: Sean Paul @ 2018-09-19 19:13 UTC (permalink / raw)
To: Bruce Wang
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Wed, Sep 19, 2018 at 11:44:14AM -0400, Bruce Wang wrote:
> All checks for _dpu_crtc_power_enable are not true, so the function
> can never return an error code. All calls of the function have also
> been changed so that they don't expect a return value.
>
> Signed-off-by: Bruce Wang <bzwang@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 ++++--------------------
> 1 file changed, 5 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 017d16131dac..f0b52281c46f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -60,37 +60,16 @@ static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> return to_dpu_kms(priv->kms);
> }
>
> -static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
> +static inline void _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
> {
> - struct drm_crtc *crtc;
> - struct msm_drm_private *priv;
> - struct dpu_kms *dpu_kms;
> -
> - if (!dpu_crtc) {
> - DPU_ERROR("invalid dpu crtc\n");
> - return -EINVAL;
> - }
> -
> - crtc = &dpu_crtc->base;
> - if (!crtc->dev || !crtc->dev->dev_private) {
> - DPU_ERROR("invalid drm device\n");
> - return -EINVAL;
> - }
> -
> - priv = crtc->dev->dev_private;
> - if (!priv->kms) {
> - DPU_ERROR("invalid kms\n");
> - return -EINVAL;
> - }
> -
> - dpu_kms = to_dpu_kms(priv->kms);
> + struct drm_crtc *crtc = &dpu_crtc->base;
> + struct msm_drm_private *priv = crtc->dev->dev_private;
> + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
>
> if (enable)
> pm_runtime_get_sync(&dpu_kms->pdev->dev);
> else
> pm_runtime_put_sync(&dpu_kms->pdev->dev);
> -
> - return 0;
> }
>
> static void dpu_crtc_destroy(struct drm_crtc *crtc)
> @@ -849,14 +828,10 @@ static int _dpu_crtc_vblank_enable_no_lock(
I think this can go to void now too since the if (!dpu_crtc) check at the top is
a no-op.
This is a good thing, too since this function is only called from void functions
with one exception where its return value isn't passed on!
Sean
> dev = crtc->dev;
>
> if (enable) {
> - int ret;
> -
> /* drop lock since power crtc cb may try to re-acquire lock */
> mutex_unlock(&dpu_crtc->crtc_lock);
> - ret = _dpu_crtc_power_enable(dpu_crtc, true);
> + _dpu_crtc_power_enable(dpu_crtc, true);
> mutex_lock(&dpu_crtc->crtc_lock);
> - if (ret)
> - return ret;
>
> list_for_each_entry(enc, &dev->mode_config.encoder_list, head) {
> if (enc->crtc != crtc)
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock to void
[not found] ` <20180919154415.131292-6-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2018-09-19 19:14 ` Sean Paul
0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2018-09-19 19:14 UTC (permalink / raw)
To: Bruce Wang
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Wed, Sep 19, 2018 at 11:44:15AM -0400, Bruce Wang wrote:
> Removes redundant tests for _dpu_crtc_vblank_enable_no_lock.
> Function return type is now void and all function calls have
> been changed accordingly.
Ha! Glad we're on the same page, this will teach me to review sequentially.
Reviewed-by: Sean Paul <seanpaul@chromium.org>
>
> Signed-off-by: Bruce Wang <bzwang@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 42 ++++--------------------
> 1 file changed, 7 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index f0b52281c46f..8ce24a85e70b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -809,24 +809,14 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> * _dpu_crtc_vblank_enable_no_lock - update power resource and vblank request
> * @dpu_crtc: Pointer to dpu crtc structure
> * @enable: Whether to enable/disable vblanks
> - *
> - * @Return: error code
> */
> -static int _dpu_crtc_vblank_enable_no_lock(
> +static void _dpu_crtc_vblank_enable_no_lock(
> struct dpu_crtc *dpu_crtc, bool enable)
> {
> - struct drm_device *dev;
> - struct drm_crtc *crtc;
> + struct drm_crtc *crtc = &dpu_crtc->base;
> + struct drm_device *dev = crtc->dev;
> struct drm_encoder *enc;
>
> - if (!dpu_crtc) {
> - DPU_ERROR("invalid crtc\n");
> - return -EINVAL;
> - }
> -
> - crtc = &dpu_crtc->base;
> - dev = crtc->dev;
> -
> if (enable) {
> /* drop lock since power crtc cb may try to re-acquire lock */
> mutex_unlock(&dpu_crtc->crtc_lock);
> @@ -861,8 +851,6 @@ static int _dpu_crtc_vblank_enable_no_lock(
> _dpu_crtc_power_enable(dpu_crtc, false);
> mutex_lock(&dpu_crtc->crtc_lock);
> }
> -
> - return 0;
> }
>
> /**
> @@ -873,7 +861,6 @@ static int _dpu_crtc_vblank_enable_no_lock(
> static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
> {
> struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> - int ret = 0;
>
> DRM_DEBUG_KMS("crtc%d suspend = %d\n", crtc->base.id, enable);
>
> @@ -888,10 +875,7 @@ static void _dpu_crtc_set_suspend(struct drm_crtc *crtc, bool enable)
> DPU_DEBUG("crtc%d suspend already set to %d, ignoring update\n",
> crtc->base.id, enable);
> else if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
> - ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
> - if (ret)
> - DPU_ERROR("%s vblank enable failed: %d\n",
> - dpu_crtc->name, ret);
> + _dpu_crtc_vblank_enable_no_lock(dpu_crtc, !enable);
> }
>
> dpu_crtc->suspend = enable;
> @@ -1000,7 +984,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> struct drm_display_mode *mode;
> struct drm_encoder *encoder;
> struct msm_drm_private *priv;
> - int ret;
> unsigned long flags;
>
> if (!crtc || !crtc->dev || !crtc->dev->dev_private || !crtc->state) {
> @@ -1031,10 +1014,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
> if (dpu_crtc->enabled && !dpu_crtc->suspend &&
> dpu_crtc->vblank_requested) {
> - ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
> - if (ret)
> - DPU_ERROR("%s vblank enable failed: %d\n",
> - dpu_crtc->name, ret);
> + _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
> }
> dpu_crtc->enabled = false;
>
> @@ -1080,7 +1060,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
> struct dpu_crtc *dpu_crtc;
> struct drm_encoder *encoder;
> struct msm_drm_private *priv;
> - int ret;
>
> if (!crtc || !crtc->dev || !crtc->dev->dev_private) {
> DPU_ERROR("invalid crtc\n");
> @@ -1102,10 +1081,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
> trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
> if (!dpu_crtc->enabled && !dpu_crtc->suspend &&
> dpu_crtc->vblank_requested) {
> - ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
> - if (ret)
> - DPU_ERROR("%s vblank enable failed: %d\n",
> - dpu_crtc->name, ret);
> + _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
> }
> dpu_crtc->enabled = true;
>
> @@ -1360,7 +1336,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
> int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> {
> struct dpu_crtc *dpu_crtc;
> - int ret;
>
> if (!crtc) {
> DPU_ERROR("invalid crtc\n");
> @@ -1371,10 +1346,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
> mutex_lock(&dpu_crtc->crtc_lock);
> trace_dpu_crtc_vblank(DRMID(&dpu_crtc->base), en, dpu_crtc);
> if (dpu_crtc->enabled && !dpu_crtc->suspend) {
> - ret = _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
> - if (ret)
> - DPU_ERROR("%s vblank enable failed: %d\n",
> - dpu_crtc->name, ret);
> + _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
> }
> dpu_crtc->vblank_requested = en;
> mutex_unlock(&dpu_crtc->crtc_lock);
> --
> 2.19.0.397.gdd90340f6a-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void
2018-09-19 19:13 ` Sean Paul
@ 2018-09-19 19:14 ` Sean Paul
0 siblings, 0 replies; 12+ messages in thread
From: Sean Paul @ 2018-09-19 19:14 UTC (permalink / raw)
To: Bruce Wang
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w,
seanpaul-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Wed, Sep 19, 2018 at 03:13:12PM -0400, Sean Paul wrote:
> On Wed, Sep 19, 2018 at 11:44:14AM -0400, Bruce Wang wrote:
> > All checks for _dpu_crtc_power_enable are not true, so the function
> > can never return an error code. All calls of the function have also
> > been changed so that they don't expect a return value.
> >
> > Signed-off-by: Bruce Wang <bzwang@chromium.org>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 35 ++++--------------------
> > 1 file changed, 5 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 017d16131dac..f0b52281c46f 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -60,37 +60,16 @@ static inline struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> > return to_dpu_kms(priv->kms);
> > }
> >
> > -static inline int _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
> > +static inline void _dpu_crtc_power_enable(struct dpu_crtc *dpu_crtc, bool enable)
> > {
> > - struct drm_crtc *crtc;
> > - struct msm_drm_private *priv;
> > - struct dpu_kms *dpu_kms;
> > -
> > - if (!dpu_crtc) {
> > - DPU_ERROR("invalid dpu crtc\n");
> > - return -EINVAL;
> > - }
> > -
> > - crtc = &dpu_crtc->base;
> > - if (!crtc->dev || !crtc->dev->dev_private) {
> > - DPU_ERROR("invalid drm device\n");
> > - return -EINVAL;
> > - }
> > -
> > - priv = crtc->dev->dev_private;
> > - if (!priv->kms) {
> > - DPU_ERROR("invalid kms\n");
> > - return -EINVAL;
> > - }
> > -
> > - dpu_kms = to_dpu_kms(priv->kms);
> > + struct drm_crtc *crtc = &dpu_crtc->base;
> > + struct msm_drm_private *priv = crtc->dev->dev_private;
> > + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> >
> > if (enable)
> > pm_runtime_get_sync(&dpu_kms->pdev->dev);
> > else
> > pm_runtime_put_sync(&dpu_kms->pdev->dev);
> > -
> > - return 0;
> > }
> >
> > static void dpu_crtc_destroy(struct drm_crtc *crtc)
> > @@ -849,14 +828,10 @@ static int _dpu_crtc_vblank_enable_no_lock(
>
> I think this can go to void now too since the if (!dpu_crtc) check at the top is
> a no-op.
>
> This is a good thing, too since this function is only called from void functions
> with one exception where its return value isn't passed on!
In light of patch 5/5,
Reviewed-by: Sean Paul <seanpaul@chromium.org>
>
> Sean
>
> > dev = crtc->dev;
> >
> > if (enable) {
> > - int ret;
> > -
> > /* drop lock since power crtc cb may try to re-acquire lock */
> > mutex_unlock(&dpu_crtc->crtc_lock);
> > - ret = _dpu_crtc_power_enable(dpu_crtc, true);
> > + _dpu_crtc_power_enable(dpu_crtc, true);
> > mutex_lock(&dpu_crtc->crtc_lock);
> > - if (ret)
> > - return ret;
> >
> > list_for_each_entry(enc, &dev->mode_config.encoder_list, head) {
> > if (enc->crtc != crtc)
> > --
> > 2.19.0.397.gdd90340f6a-goog
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-09-19 19:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 15:44 [PATCH 0/5] drm/msm/dpu: Clean up dpu code Bruce Wang
[not found] ` <20180919154415.131292-1-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 15:44 ` [PATCH 1/5] drm/msm/dpu: Remove unneeded checks from dpu_plane Bruce Wang
[not found] ` <20180919154415.131292-2-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 19:01 ` Sean Paul
2018-09-19 15:44 ` [PATCH 2/5] drm/msm/dpu: Clean up plane atomic disable/update Bruce Wang
[not found] ` <20180919154415.131292-3-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 19:02 ` Sean Paul
2018-09-19 15:44 ` [PATCH 3/5] drm/msm/dpu: Remove impossible checks Bruce Wang
[not found] ` <20180919154415.131292-4-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 19:10 ` Sean Paul
2018-09-19 15:44 ` [PATCH 4/5] drm/msm/dpu: Change _dpu_crtc_power_enable to void Bruce Wang
[not found] ` <20180919154415.131292-5-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 19:13 ` Sean Paul
2018-09-19 19:14 ` Sean Paul
2018-09-19 15:44 ` [PATCH 5/5] drm/msm/dpu: Change _dpu_crtc_vblank_enable_no_lock " Bruce Wang
[not found] ` <20180919154415.131292-6-bzwang-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-09-19 19:14 ` Sean Paul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.