drm/msm: mdp5+dpu interconnect support
Subsystems (5)
cover drm/msm: mdp5+dpu interconnect support
From: Rob Clark <[email protected]>
Most of this is a resend of things that have already been posted to list. I've rebased the DPU patches, which was somewhat conflicty due to other changes and refactoring in the DPU code.
Abhinav Kumar (1):
drm/msm/dpu: add icc voting in dpu_mdss_init
Georgi Djakov (1):
drm/msm/mdp5: Use the interconnect API
Jayant Shekhar (3):
drm/msm/dpu: clean up references of DPU custom bus scaling drm/msm/dpu: Integrate interconnect API in MDSS
dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on
SDM845
.../devicetree/bindings/display/msm/dpu.txt | 10 + drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++++++----------- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 +- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 57 +++++- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 22 +-- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++ 7 files changed, 158 insertions(+), 134 deletions(-)
Signature · 6 lines
2.20.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/5 drm/msm/dpu: clean up references of DPU custom bus scaling
From: Jayant Shekhar <[email protected]>
Since the upstream interconnect bus framework has landed upstream, the existing references of custom bus scaling needs to be cleaned up.
Changes in v2:
- Fixed build error due to partial clean up
Changes in v3:
- Condense multiple lines into a single line (Sean Paul)
Changes in v4-v7:
- None
--- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++++++----------- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 +- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 22 +-- 4 files changed, 80 insertions(+), 131 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +68 -106
@@ -77,7 +77,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms, struct dpu_core_perf_params *perf) {struct dpu_crtc_state *dpu_cstate; -int i;if (!kms || !kms->catalog || !crtc || !state || !perf) {DPU_ERROR ("invalid parameters\n" );
@@ -88,35 +87,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,memset (perf,0 ,sizeof (struct dpu_core_perf_params ));if (!dpu_cstate->bw_control) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - perf->bw_ctl[i] = kms->catalog->perf.max_bw_high * + perf->bw_ctl = kms->catalog->perf.max_bw_high *1000ULL ; - perf->max_per_pipe_ib[i] = perf->bw_ctl[i]; - } + perf->max_per_pipe_ib = perf->bw_ctl; perf->core_clk_rate = kms->perf.max_core_clk_rate; }else if (kms->perf.perf_tune.mode ==DPU_PERF_MODE_MINIMUM ) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - perf->bw_ctl[i] =0 ; - perf->max_per_pipe_ib[i] =0 ; - } + perf->bw_ctl =0 ; + perf->max_per_pipe_ib =0 ; perf->core_clk_rate =0 ; }else if (kms->perf.perf_tune.mode ==DPU_PERF_MODE_FIXED ) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - perf->bw_ctl[i] = kms->perf.fix_core_ab_vote; - perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote; - } + perf->bw_ctl = kms->perf.fix_core_ab_vote; + perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote; perf->core_clk_rate = kms->perf.fix_core_clk_rate; }DPU_DEBUG ( -"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n" , +"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n" , crtc->base.id, perf->core_clk_rate, - perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC ], - perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC ], - perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC ], - perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC ], - perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI ], - perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI ]); + perf->max_per_pipe_ib, perf->bw_ctl); } int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -129,7 +117,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,struct dpu_crtc_state *dpu_cstate;struct drm_crtc *tmp_crtc;struct dpu_kms *kms; -int i;if (!crtc || !state) {DPU_ERROR ("invalid crtc\n" );
@@ -151,31 +138,25 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,/* obtain new values */ _dpu_core_perf_calc_crtc (kms, crtc, state, &dpu_cstate->new_perf); -for (i =DPU_CORE_PERF_DATA_BUS_ID_MNOC ; - i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i]; - curr_client_type =dpu_crtc_get_client_type (crtc); + bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl; + curr_client_type =dpu_crtc_get_client_type (crtc); -drm_for_each_crtc (tmp_crtc, crtc->dev) { -if (tmp_crtc->enabled && - (dpu_crtc_get_client_type (tmp_crtc) == - curr_client_type) && - (tmp_crtc != crtc)) { -struct dpu_crtc_state *tmp_cstate = -to_dpu_crtc_state (tmp_crtc->state); - -DPU_DEBUG ("crtc:%d bw:%llu ctrl:%d\n" , - tmp_crtc->base.id, - tmp_cstate->new_perf.bw_ctl[i], - tmp_cstate->bw_control); -/* -* For bw check only use the bw if the -* atomic property has been already set -*/ -if (tmp_cstate->bw_control) - bw_sum_of_intfs += - tmp_cstate->new_perf.bw_ctl[i]; - } +drm_for_each_crtc (tmp_crtc, crtc->dev) { +if (tmp_crtc->enabled && + (dpu_crtc_get_client_type (tmp_crtc) == + curr_client_type) && (tmp_crtc != crtc)) { +struct dpu_crtc_state *tmp_cstate = +to_dpu_crtc_state (tmp_crtc->state); + +DPU_DEBUG ("crtc:%d bw:%llu ctrl:%d\n" , + tmp_crtc->base.id, tmp_cstate->new_perf.bw_ctl, + tmp_cstate->bw_control); +/* +* For bw check only use the bw if the +* atomic property has been already set +*/ +if (tmp_cstate->bw_control) + bw_sum_of_intfs += tmp_cstate->new_perf.bw_ctl; }/* convert bandwidth to kb */
@@ -206,9 +187,9 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc, }static int _dpu_core_perf_crtc_update_bus (struct dpu_kms *kms, -struct drm_crtc *crtc,u32 bus_id) +struct drm_crtc *crtc) { -struct dpu_core_perf_params perf = { {0 } }; +struct dpu_core_perf_params perf = {0 };enum dpu_crtc_client_type curr_client_type =dpu_crtc_get_client_type (crtc);struct drm_crtc *tmp_crtc;
@@ -221,13 +202,11 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms, dpu_crtc_get_client_type(tmp_crtc)) { dpu_cstate =to_dpu_crtc_state (tmp_crtc->state); - perf.max_per_pipe_ib[bus_id] = -max (perf.max_per_pipe_ib[bus_id], - dpu_cstate->new_perf.max_per_pipe_ib[bus_id]); + perf.max_per_pipe_ib =max (perf.max_per_pipe_ib, + dpu_cstate->new_perf.max_per_pipe_ib); -DPU_DEBUG ("crtc=%d bus_id=%d bw=%llu\n" , - tmp_crtc->base.id, bus_id, - dpu_cstate->new_perf.bw_ctl[bus_id]); +DPU_DEBUG ("crtc=%d bw=%llu\n" , tmp_crtc->base.id, + dpu_cstate->new_perf.bw_ctl); } }return ret;
@@ -247,7 +226,6 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)struct dpu_crtc *dpu_crtc;struct dpu_crtc_state *dpu_cstate;struct dpu_kms *kms; -int i;if (!crtc) {DPU_ERROR ("invalid crtc\n" );
@@ -283,10 +261,8 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)if (kms->perf.enable_bw_release) {trace_dpu_cmd_release_bw (crtc->base.id);DPU_DEBUG ("Release BW crtc=%d\n" , crtc->base.id); -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - dpu_crtc->cur_perf.bw_ctl[i] =0 ; -_dpu_core_perf_crtc_update_bus (kms, crtc, i); - } + dpu_crtc->cur_perf.bw_ctl =0 ; +_dpu_core_perf_crtc_update_bus (kms, crtc); } }
@@ -329,11 +305,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc, int params_changed, bool stop_req) {struct dpu_core_perf_params *new, *old; -int update_bus =0 , update_clk =0 ; +bool update_bus = false, update_clk = false;u64 clk_rate =0 ;struct dpu_crtc *dpu_crtc;struct dpu_crtc_state *dpu_cstate; -int i;struct msm_drm_private *priv;struct dpu_kms *kms;int ret;
@@ -360,62 +335,49 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc, new = &dpu_cstate->new_perf;if (crtc->enabled && !stop_req) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { -/* -* cases for bus bandwidth update. -* 1. new bandwidth vote - "ab or ib vote" is higher -* than current vote for update request. -* 2. new bandwidth vote - "ab or ib vote" is lower -* than current vote at end of commit or stop. -*/ -if ((params_changed && ((new->bw_ctl[i] > - old->bw_ctl[i]) || - (new->max_per_pipe_ib[i] > - old->max_per_pipe_ib[i]))) || - (!params_changed && ((new->bw_ctl[i] < - old->bw_ctl[i]) || - (new->max_per_pipe_ib[i] < - old->max_per_pipe_ib[i])))) { -DPU_DEBUG ( -"crtc=%d p=%d new_bw=%llu,old_bw=%llu\n" , - crtc->base.id, params_changed, - new->bw_ctl[i], old->bw_ctl[i]); - old->bw_ctl[i] = new->bw_ctl[i]; - old->max_per_pipe_ib[i] = - new->max_per_pipe_ib[i]; - update_bus |=BIT (i); - } +/* +* cases for bus bandwidth update. +* 1. new bandwidth vote - "ab or ib vote" is higher +* than current vote for update request. +* 2. new bandwidth vote - "ab or ib vote" is lower +* than current vote at end of commit or stop. +*/ +if ((params_changed && ((new->bw_ctl > old->bw_ctl) || + (new->max_per_pipe_ib > old->max_per_pipe_ib))) || + (!params_changed && ((new->bw_ctl < old->bw_ctl) || + (new->max_per_pipe_ib < old->max_per_pipe_ib)))) { +DPU_DEBUG ("crtc=%d p=%d new_bw=%llu,old_bw=%llu\n" , + crtc->base.id, params_changed, + new->bw_ctl, old->bw_ctl); + old->bw_ctl = new->bw_ctl; + old->max_per_pipe_ib = new->max_per_pipe_ib; + update_bus = true; }if ((params_changed && - (new->core_clk_rate > old->core_clk_rate)) || - (!params_changed && - (new->core_clk_rate < old->core_clk_rate))) { + (new->core_clk_rate > old->core_clk_rate)) || + (!params_changed && + (new->core_clk_rate < old->core_clk_rate))) { old->core_clk_rate = new->core_clk_rate; - update_clk =1 ; + update_clk = true; } }else {DPU_DEBUG ("crtc=%d disable\n" , crtc->base.id);memset (old,0 ,sizeof (*old));memset (new,0 ,sizeof (*new)); - update_bus = ~0 ; - update_clk =1 ; + update_bus = true; + update_clk = true; } -trace_dpu_perf_crtc_update (crtc->base.id, - new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC ], - new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC ], - new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI ], - new->core_clk_rate, stop_req, - update_bus, update_clk); - -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { -if (update_bus &BIT (i)) { - ret =_dpu_core_perf_crtc_update_bus (kms, crtc, i); -if (ret) { -DPU_ERROR ("crtc-%d: failed to update bw vote for bus-%d\n" , - crtc->base.id, i); -return ret; - } + +trace_dpu_perf_crtc_update (crtc->base.id, new->bw_ctl, + new->core_clk_rate, stop_req, update_bus, update_clk); + +if (update_bus) { + ret =_dpu_core_perf_crtc_update_bus (kms, crtc); +if (ret) { +DPU_ERROR ("crtc-%d: failed to update bus bw vote\n" , + crtc->base.id); +return ret; } }
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h +2 -2
@@ -42,8 +42,8 @@ enum dpu_core_perf_data_bus_id {* @core_clk_rate: core clock rate request */ struct dpu_core_perf_params { -u64 max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MAX ]; -u64 bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MAX ]; +u64 max_per_pipe_ib; +u64 bw_ctl;u64 core_clk_rate; };
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +3 -8
@@ -1235,19 +1235,14 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v) {struct drm_crtc *crtc = (struct drm_crtc *) s->private;struct dpu_crtc *dpu_crtc =to_dpu_crtc (crtc); -int i;seq_printf (s,"client type: %d\n" ,dpu_crtc_get_client_type (crtc));seq_printf (s,"intf_mode: %d\n" ,dpu_crtc_get_intf_mode (crtc));seq_printf (s,"core_clk_rate: %llu\n" , dpu_crtc->cur_perf.core_clk_rate); -for (i =DPU_CORE_PERF_DATA_BUS_ID_MNOC ; - i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { -seq_printf (s,"bw_ctl[%d]: %llu\n" , i, - dpu_crtc->cur_perf.bw_ctl[i]); -seq_printf (s,"max_per_pipe_ib[%d]: %llu\n" , i, - dpu_crtc->cur_perf.max_per_pipe_ib[i]); - } +seq_printf (s,"bw_ctl: %llu\n" , dpu_crtc->cur_perf.bw_ctl); +seq_printf (s,"max_per_pipe_ib: %llu\n" , + dpu_crtc->cur_perf.max_per_pipe_ib);return 0 ; }
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +7 -15
@@ -146,16 +146,12 @@ TRACE_EVENT(dpu_trace_counter, ) TRACE_EVENT(dpu_perf_crtc_update, - TP_PROTO(u32 crtc, u64 bw_ctl_mnoc, u64 bw_ctl_llcc, - u64 bw_ctl_ebi, u32 core_clk_rate, - bool stop_req, u32 update_bus, u32 update_clk), - TP_ARGS(crtc, bw_ctl_mnoc, bw_ctl_llcc, bw_ctl_ebi, core_clk_rate, - stop_req, update_bus, update_clk), + TP_PROTO(u32 crtc, u64 bw_ctl, u32 core_clk_rate, + bool stop_req, bool update_bus, bool update_clk), + TP_ARGS(crtc, bw_ctl, core_clk_rate, stop_req, update_bus, update_clk), TP_STRUCT__entry( __field(u32, crtc) - __field(u64, bw_ctl_mnoc) - __field(u64, bw_ctl_llcc) - __field(u64, bw_ctl_ebi) + __field(u64, bw_ctl) __field(u32, core_clk_rate) __field(bool, stop_req) __field(u32, update_bus)
@@ -163,20 +159,16 @@ TRACE_EVENT(dpu_perf_crtc_update, ), TP_fast_assign( __entry->crtc = crtc; - __entry->bw_ctl_mnoc = bw_ctl_mnoc; - __entry->bw_ctl_llcc = bw_ctl_llcc; - __entry->bw_ctl_ebi = bw_ctl_ebi; + __entry->bw_ctl = bw_ctl; __entry->core_clk_rate = core_clk_rate; __entry->stop_req = stop_req; __entry->update_bus = update_bus; __entry->update_clk = update_clk; ), TP_printk( - "crtc=%d bw_mnoc=%llu bw_llcc=%llu bw_ebi=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d", + "crtc=%d bw_ctl=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d", __entry->crtc, - __entry->bw_ctl_mnoc, - __entry->bw_ctl_llcc, - __entry->bw_ctl_ebi, + __entry->bw_ctl, __entry->core_clk_rate, __entry->stop_req, __entry->update_bus,
Signature · 6 lines
2.20.1 _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH 1/5] drm/msm/dpu: clean up references of DPU custom bus scaling
Quoted 12 lines · Rob Clark
From: Jayant Shekhar <[email protected]>
Since the upstream interconnect bus framework has landed upstream, the existing references of custom bus scaling needs to be cleaned up.
Changes in v2:
- Fixed build error due to partial clean upChanges in v3:
- Condense multiple lines into a single line (Sean Paul)
Quoted 6 lines
Changes in v4-v7:
- NoneSigned-off-by Sravanthi Kollukuduru <[email protected]>Signed-off-by Jayant Shekhar <[email protected]>Signed-off-by Rob Clark <[email protected]>
Quoted 375 lines
--- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 174 +++++++----------- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 +- drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 22 +-- 4 files changed, 80 insertions(+), 131 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c +68
-106@@ -77,7 +77,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms, struct dpu_core_perf_params *perf) {struct dpu_crtc_state *dpu_cstate; -int i;if (!kms || !kms->catalog || !crtc || !state || !perf) {DPU_ERROR ("invalid parameters\n" );@@ -88,35 +87,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,memset (perf,0 ,sizeof (struct dpu_core_perf_params ));if (!dpu_cstate->bw_control) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - perf->bw_ctl[i] = kms->catalog->perf.max_bw_high * + perf->bw_ctl = kms->catalog->perf.max_bw_high *1000ULL ; - perf->max_per_pipe_ib[i] = perf->bw_ctl[i]; - } + perf->max_per_pipe_ib = perf->bw_ctl; perf->core_clk_rate = kms->perf.max_core_clk_rate; }else if (kms->perf.perf_tune.mode ==DPU_PERF_MODE_MINIMUM ) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - perf->bw_ctl[i] =0 ; - perf->max_per_pipe_ib[i] =0 ; - } + perf->bw_ctl =0 ; + perf->max_per_pipe_ib =0 ; perf->core_clk_rate =0 ; }else if (kms->perf.perf_tune.mode ==DPU_PERF_MODE_FIXED ) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - perf->bw_ctl[i] = kms->perf.fix_core_ab_vote; - perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote; - } + perf->bw_ctl = kms->perf.fix_core_ab_vote; + perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote; perf->core_clk_rate = kms->perf.fix_core_clk_rate; }DPU_DEBUG ( -"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n" , +"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n" , crtc->base.id, perf->core_clk_rate, - perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC ], - perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC ], - perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC ], - perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC ], - perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI ], - perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI ]); + perf->max_per_pipe_ib, perf->bw_ctl); } int dpu_core_perf_crtc_check(struct drm_crtc *crtc,@@ -129,7 +117,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,struct dpu_crtc_state *dpu_cstate;struct drm_crtc *tmp_crtc;struct dpu_kms *kms; -int i;if (!crtc || !state) {DPU_ERROR ("invalid crtc\n" );@@ -151,31 +138,25 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,/* obtain new values */ _dpu_core_perf_calc_crtc (kms, crtc, state, &dpu_cstate->new_perf); -for (i =DPU_CORE_PERF_DATA_BUS_ID_MNOC ; - i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i]; - curr_client_type =dpu_crtc_get_client_type (crtc); + bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl; + curr_client_type =dpu_crtc_get_client_type (crtc); -drm_for_each_crtc (tmp_crtc, crtc->dev) { -if (tmp_crtc->enabled && - (dpu_crtc_get_client_type (tmp_crtc) == - curr_client_type) && - (tmp_crtc != crtc)) { -struct dpu_crtc_state *tmp_cstate = -to_dpu_crtc_state (tmp_crtc->state); - -DPU_DEBUG ("crtc:%d bw:%llu ctrl:%d\n" , - tmp_crtc->base.id, - tmp_cstate->new_perf.bw_ctl[i], - tmp_cstate->bw_control); -/* -* For bw check only use the bw if the -* atomic property has been already set -*/ -if (tmp_cstate->bw_control) - bw_sum_of_intfs += - tmp_cstate->new_perf.bw_ctl[i]; - } +drm_for_each_crtc (tmp_crtc, crtc->dev) { +if (tmp_crtc->enabled && + (dpu_crtc_get_client_type (tmp_crtc) == + curr_client_type) && (tmp_crtc != crtc)) { +struct dpu_crtc_state *tmp_cstate = +to_dpu_crtc_state (tmp_crtc->state); + +DPU_DEBUG ("crtc:%d bw:%llu ctrl:%d\n" , + tmp_crtc->base.id, tmp_cstate->new_perf.bw_ctl, + tmp_cstate->bw_control); +/* +* For bw check only use the bw if the +* atomic property has been already set +*/ +if (tmp_cstate->bw_control) + bw_sum_of_intfs += tmp_cstate->new_perf.bw_ctl; }/* convert bandwidth to kb */ @@ -206,9 +187,9 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc, }static int _dpu_core_perf_crtc_update_bus (struct dpu_kms *kms, -struct drm_crtc *crtc,u32 bus_id) +struct drm_crtc *crtc) { -struct dpu_core_perf_params perf = { {0 } }; +struct dpu_core_perf_params perf = {0 };enum dpu_crtc_client_type curr_client_type =dpu_crtc_get_client_type (crtc);struct drm_crtc *tmp_crtc;@@ -221,13 +202,11 @@ static int _dpu_core_perf_crtc_update_bus(struct dpu_kms *kms, dpu_crtc_get_client_type(tmp_crtc)) { dpu_cstate =to_dpu_crtc_state (tmp_crtc->state); - perf.max_per_pipe_ib[bus_id] = -max (perf.max_per_pipe_ib[bus_id], - dpu_cstate->new_perf.max_per_pipe_ib[bus_id]); + perf.max_per_pipe_ib =max (perf.max_per_pipe_ib, + dpu_cstate->new_perf.max_per_pipe_ib); -DPU_DEBUG ("crtc=%d bus_id=%d bw=%llu\n" , - tmp_crtc->base.id, bus_id, - dpu_cstate->new_perf.bw_ctl[bus_id]); +DPU_DEBUG ("crtc=%d bw=%llu\n" , tmp_crtc->base.id, + dpu_cstate->new_perf.bw_ctl); } }return ret;@@ -247,7 +226,6 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)struct dpu_crtc *dpu_crtc;struct dpu_crtc_state *dpu_cstate;struct dpu_kms *kms; -int i;if (!crtc) {DPU_ERROR ("invalid crtc\n" );@@ -283,10 +261,8 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)if (kms->perf.enable_bw_release) {trace_dpu_cmd_release_bw (crtc->base.id);DPU_DEBUG ("Release BW crtc=%d\n" , crtc->base.id); -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { - dpu_crtc->cur_perf.bw_ctl[i] =0 ; -_dpu_core_perf_crtc_update_bus (kms, crtc, i); - } + dpu_crtc->cur_perf.bw_ctl =0 ; +_dpu_core_perf_crtc_update_bus (kms, crtc); } }@@ -329,11 +305,10 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc, int params_changed, bool stop_req) {struct dpu_core_perf_params *new, *old; -int update_bus =0 , update_clk =0 ; +bool update_bus = false, update_clk = false;u64 clk_rate =0 ;struct dpu_crtc *dpu_crtc;struct dpu_crtc_state *dpu_cstate; -int i;struct msm_drm_private *priv;struct dpu_kms *kms;int ret;@@ -360,62 +335,49 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc, new = &dpu_cstate->new_perf;if (crtc->enabled && !stop_req) { -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { -/* -* cases for bus bandwidth update. -* 1. new bandwidth vote - "ab or ib vote" is higher -* than current vote for update request. -* 2. new bandwidth vote - "ab or ib vote" is lower -* than current vote at end of commit or stop. -*/ -if ((params_changed && ((new->bw_ctl[i] > - old->bw_ctl[i]) || - (new->max_per_pipe_ib[i] > - old->max_per_pipe_ib[i]))) || - (!params_changed && ((new->bw_ctl[i] < - old->bw_ctl[i]) || - (new->max_per_pipe_ib[i] < - old->max_per_pipe_ib[i])))) { -DPU_DEBUG ( -"crtc=%d p=%d new_bw=%llu,old_bw=%llu\n" , - crtc->base.id, params_changed, - new->bw_ctl[i], old->bw_ctl[i]); - old->bw_ctl[i] = new->bw_ctl[i]; - old->max_per_pipe_ib[i] = - new->max_per_pipe_ib[i]; - update_bus |=BIT (i); - } +/* +* cases for bus bandwidth update. +* 1. new bandwidth vote - "ab or ib vote" is higher +* than current vote for update request. +* 2. new bandwidth vote - "ab or ib vote" is lower +* than current vote at end of commit or stop. +*/ +if ((params_changed && ((new->bw_ctl > old->bw_ctl) || + (new->max_per_pipe_ib > old->max_per_pipe_ib))) || + (!params_changed && ((new->bw_ctl < old->bw_ctl) || + (new->max_per_pipe_ib < old->max_per_pipe_ib)))) { +DPU_DEBUG ("crtc=%d p=%d new_bw=%llu,old_bw=%llu\n" , + crtc->base.id, params_changed, + new->bw_ctl, old->bw_ctl); + old->bw_ctl = new->bw_ctl; + old->max_per_pipe_ib = new->max_per_pipe_ib; + update_bus = true; }if ((params_changed && - (new->core_clk_rate > old->core_clk_rate)) || - (!params_changed && - (new->core_clk_rate < old->core_clk_rate))) { + (new->core_clk_rate > old->core_clk_rate)) || + (!params_changed && + (new->core_clk_rate < old->core_clk_rate))) { old->core_clk_rate = new->core_clk_rate; - update_clk =1 ; + update_clk = true; } }else {DPU_DEBUG ("crtc=%d disable\n" , crtc->base.id);memset (old,0 ,sizeof (*old));memset (new,0 ,sizeof (*new)); - update_bus = ~0 ; - update_clk =1 ; + update_bus = true; + update_clk = true; } -trace_dpu_perf_crtc_update (crtc->base.id, - new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC ], - new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC ], - new->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI ], - new->core_clk_rate, stop_req, - update_bus, update_clk); - -for (i =0 ; i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { -if (update_bus &BIT (i)) { - ret =_dpu_core_perf_crtc_update_bus (kms, crtc, i); -if (ret) { -DPU_ERROR ("crtc-%d: failed to update bw vote for bus-%d\n" , - crtc->base.id, i); -return ret; - } + +trace_dpu_perf_crtc_update (crtc->base.id, new->bw_ctl, + new->core_clk_rate, stop_req, update_bus, update_clk); + +if (update_bus) { + ret =_dpu_core_perf_crtc_update_bus (kms, crtc); +if (ret) { +DPU_ERROR ("crtc-%d: failed to update bus bw vote\n" , + crtc->base.id); +return ret; } }drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h +2
-2@@ -42,8 +42,8 @@ enum dpu_core_perf_data_bus_id {* @core_clk_rate: core clock rate request */ struct dpu_core_perf_params { -u64 max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MAX ]; -u64 bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MAX ]; +u64 max_per_pipe_ib; +u64 bw_ctl;u64 core_clk_rate; };drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +3
-8@@ -1235,19 +1235,14 @@ static int dpu_crtc_debugfs_state_show(struct seq_file *s, void *v) {struct drm_crtc *crtc = (struct drm_crtc *) s->private;struct dpu_crtc *dpu_crtc =to_dpu_crtc (crtc); -int i;seq_printf (s,"client type: %d\n" ,dpu_crtc_get_client_type (crtc));seq_printf (s,"intf_mode: %d\n" ,dpu_crtc_get_intf_mode (crtc));seq_printf (s,"core_clk_rate: %llu\n" , dpu_crtc->cur_perf.core_clk_rate); -for (i =DPU_CORE_PERF_DATA_BUS_ID_MNOC ; - i <DPU_CORE_PERF_DATA_BUS_ID_MAX ; i++) { -seq_printf (s,"bw_ctl[%d]: %llu\n" , i, - dpu_crtc->cur_perf.bw_ctl[i]); -seq_printf (s,"max_per_pipe_ib[%d]: %llu\n" , i, - dpu_crtc->cur_perf.max_per_pipe_ib[i]); - } +seq_printf (s,"bw_ctl: %llu\n" , dpu_crtc->cur_perf.bw_ctl); +seq_printf (s,"max_per_pipe_ib: %llu\n" , + dpu_crtc->cur_perf.max_per_pipe_ib);return 0 ; }drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h +7
-15@@ -146,16 +146,12 @@ TRACE_EVENT(dpu_trace_counter, ) TRACE_EVENT(dpu_perf_crtc_update, - TP_PROTO(u32 crtc, u64 bw_ctl_mnoc, u64 bw_ctl_llcc, - u64 bw_ctl_ebi, u32 core_clk_rate, - bool stop_req, u32 update_bus, u32 update_clk), - TP_ARGS(crtc, bw_ctl_mnoc, bw_ctl_llcc, bw_ctl_ebi, core_clk_rate, - stop_req, update_bus, update_clk), + TP_PROTO(u32 crtc, u64 bw_ctl, u32 core_clk_rate, + bool stop_req, bool update_bus, bool update_clk), + TP_ARGS(crtc, bw_ctl, core_clk_rate, stop_req, update_bus, update_clk), TP_STRUCT__entry( __field(u32, crtc) - __field(u64, bw_ctl_mnoc) - __field(u64, bw_ctl_llcc) - __field(u64, bw_ctl_ebi) + __field(u64, bw_ctl) __field(u32, core_clk_rate) __field(bool, stop_req) __field(u32, update_bus)@@ -163,20 +159,16 @@ TRACE_EVENT(dpu_perf_crtc_update, ), TP_fast_assign( __entry->crtc = crtc; - __entry->bw_ctl_mnoc = bw_ctl_mnoc; - __entry->bw_ctl_llcc = bw_ctl_llcc; - __entry->bw_ctl_ebi = bw_ctl_ebi; + __entry->bw_ctl = bw_ctl; __entry->core_clk_rate = core_clk_rate; __entry->stop_req = stop_req; __entry->update_bus = update_bus; __entry->update_clk = update_clk; ), TP_printk( - "crtc=%d bw_mnoc=%llu bw_llcc=%llu bw_ebi=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d", + "crtc=%d bw_ctl=%llu clk_rate=%u stop_req=%d u_bus=%d u_clk=%d", __entry->crtc, - __entry->bw_ctl_mnoc, - __entry->bw_ctl_llcc, - __entry->bw_ctl_ebi, + __entry->bw_ctl, __entry->core_clk_rate, __entry->stop_req, __entry->update_bus,Signature · 1 line
2.20.1
Signature · 1 line
Sean Paul, Software Engineer, Google / Chromium OS
PATCH 2/5 drm/msm/dpu: Integrate interconnect API in MDSS
In reply to Rob ClarkFrom: Jayant Shekhar <[email protected]>
The interconnect framework is designed to provide a standard kernel interface to control the settings of the interconnects on a SoC.
The interconnect API uses a consumer/provider-based model, where the providers are the interconnect buses and the consumers could be various drivers.
MDSS is one of the interconnect consumers which uses the interconnect APIs to get the path between endpoints and set its bandwidth requirement for the given interconnected path.
Changes in v2:
- Remove error log and unnecessary check (Jordan Crouse)
Changes in v3:
- Code clean involving variable name change, removal of extra paranthesis and variables (Matthias Kaehlcke)
Changes in v4:
- Add comments, spacings, tabs, proper port name and icc macro (Georgi Djakov)
Changes in v5:
- Commit text and parenthesis alignment (Georgi Djakov)
Changes in v6:
- Change to new icc_set API's (Doug Anderson)
Changes in v7:
- Fixed a typo
Changes in v8:
- Handle the of_icc_get() returning NULL case. In practice icc_set_bw() will gracefully handle the case of a NULL path, but it's probably best for clarity to keep num_paths=0 in this case.
--- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 ++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +45 -4
@@ -4,11 +4,15 @@*/ #include "dpu_kms.h" +#include <linux/interconnect.h> #define to_dpu_mdss (x) container_of(x, struct dpu_mdss, base)#define HW_INTR_STATUS 0x0010 +/* Max BW defined in KBps */ +#define MAX_BW 6800000 +struct dpu_irq_controller {unsigned long enabled_mask;struct irq_domain *domain;
@@ -21,8 +25,30 @@ struct dpu_mdss {u32 hwversion;struct dss_module_power mp;struct dpu_irq_controller irq_controller; +struct icc_path *path[2 ]; +u32 num_paths; }; +static int dpu_mdss_parse_data_bus_icc_path (struct drm_device *dev, +struct dpu_mdss *dpu_mdss) +{ +struct icc_path *path0 =of_icc_get (dev->dev,"mdp0-mem" ); +struct icc_path *path1 =of_icc_get (dev->dev,"mdp1-mem" ); + +if (IS_ERR_OR_NULL (path0)) +return PTR_ERR_OR_ZERO (path0); + + dpu_mdss->path[0 ] = path0; + dpu_mdss->num_paths =1 ; + +if (!IS_ERR_OR_NULL (path1)) { + dpu_mdss->path[1 ] = path1; + dpu_mdss->num_paths++; + } + +return 0 ; +} +static void dpu_mdss_irq (struct irq_desc *desc) {struct dpu_mdss *dpu_mdss =irq_desc_get_handler_data (desc);
@@ -134,7 +160,11 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) {struct dpu_mdss *dpu_mdss =to_dpu_mdss (mdss);struct dss_module_power *mp = &dpu_mdss->mp; -int ret; +int ret, i; +u64 avg_bw = dpu_mdss->num_paths ?MAX_BW / dpu_mdss->num_paths :0 ; + +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_set_bw (dpu_mdss->path[i], avg_bw,kBps_to_icc (MAX_BW )); ret =msm_dss_enable_clk (mp->clk_config, mp->num_clk, true);if (ret)
@@ -147,12 +177,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss) {struct dpu_mdss *dpu_mdss =to_dpu_mdss (mdss);struct dss_module_power *mp = &dpu_mdss->mp; -int ret; +int ret, i; ret =msm_dss_enable_clk (mp->clk_config, mp->num_clk, false);if (ret)DPU_ERROR ("clock disable failed, ret:%d\n" , ret); +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_set_bw (dpu_mdss->path[i],0 ,0 ); +return ret; }
@@ -163,6 +196,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)struct dpu_mdss *dpu_mdss =to_dpu_mdss (priv->mdss);struct dss_module_power *mp = &dpu_mdss->mp;int irq; +int i;pm_runtime_suspend (dev->dev);pm_runtime_disable (dev->dev);
@@ -172,6 +206,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)msm_dss_put_clk (mp->clk_config, mp->num_clk);devm_kfree (&pdev->dev, mp->clk_config); +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_put (dpu_mdss->path[i]); +if (dpu_mdss->mmio)devm_iounmap (&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio =NULL ;
@@ -211,6 +248,10 @@ int dpu_mdss_init(struct drm_device *dev) } dpu_mdss->mmio_len =resource_size (res); + ret =dpu_mdss_parse_data_bus_icc_path (dev, dpu_mdss); +if (ret) +return ret; + mp = &dpu_mdss->mp; ret =msm_dss_parse_clock (pdev, mp);if (ret) {
@@ -232,14 +273,14 @@ int dpu_mdss_init(struct drm_device *dev)irq_set_chained_handler_and_data (irq, dpu_mdss_irq, dpu_mdss); + priv->mdss = &dpu_mdss->base; +pm_runtime_enable (dev->dev);pm_runtime_get_sync (dev->dev); dpu_mdss->hwversion =readl_relaxed (dpu_mdss->mmio);pm_runtime_put_sync (dev->dev); - priv->mdss = &dpu_mdss->base; -return ret; irq_error:
Signature · 6 lines
2.20.1 _______________________________________________ Freedreno mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [PATCH 2/5] drm/msm/dpu: Integrate interconnect API in MDSS
Quoted 41 lines · Rob Clark
From: Jayant Shekhar <[email protected]>
The interconnect framework is designed to provide a standard kernel interface to control the settings of the interconnects on a SoC.
The interconnect API uses a consumer/provider-based model, where the providers are the interconnect buses and the consumers could be various drivers.
MDSS is one of the interconnect consumers which uses the interconnect APIs to get the path between endpoints and set its bandwidth requirement for the given interconnected path.
Changes in v2:
- Remove error log and unnecessary check (Jordan Crouse)Changes in v3:
- Code clean involving variable name change, removal of extra paranthesis and variables (Matthias Kaehlcke)Changes in v4:
- Add comments, spacings, tabs, proper port name and icc macro (Georgi Djakov)Changes in v5:
- Commit text and parenthesis alignment (Georgi Djakov)Changes in v6:
- Change to newicc_setAPI's (Doug Anderson)Changes in v7:
- Fixed a typoChanges in v8:
- Handle theof_icc_get()returning NULL case. In practiceicc_set_bw()will gracefully handle the case of a NULL path, but it's probably best for clarity to keep num_paths=0 in this case.
Quoted 4 lines
Signed-off-by Sravanthi Kollukuduru <[email protected]>Signed-off-by Jayant Shekhar <[email protected]>Signed-off-by Rob Clark <[email protected]>Acked-by Georgi Djakov <[email protected]>
Quoted 134 lines
--- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 49 ++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-)drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +45
-4@@ -4,11 +4,15 @@*/ #include "dpu_kms.h" +#include <linux/interconnect.h> #define to_dpu_mdss (x) container_of(x, struct dpu_mdss, base)#define HW_INTR_STATUS 0x0010 +/* Max BW defined in KBps */ +#define MAX_BW 6800000 +struct dpu_irq_controller {unsigned long enabled_mask;struct irq_domain *domain;@@ -21,8 +25,30 @@ struct dpu_mdss {u32 hwversion;struct dss_module_power mp;struct dpu_irq_controller irq_controller; +struct icc_path *path[2 ]; +u32 num_paths; }; +static int dpu_mdss_parse_data_bus_icc_path (struct drm_device *dev, +struct dpu_mdss *dpu_mdss) +{ +struct icc_path *path0 =of_icc_get (dev->dev,"mdp0-mem" ); +struct icc_path *path1 =of_icc_get (dev->dev,"mdp1-mem" ); + +if (IS_ERR_OR_NULL (path0)) +return PTR_ERR_OR_ZERO (path0); + + dpu_mdss->path[0 ] = path0; + dpu_mdss->num_paths =1 ; + +if (!IS_ERR_OR_NULL (path1)) { + dpu_mdss->path[1 ] = path1; + dpu_mdss->num_paths++; + } + +return 0 ; +} +static void dpu_mdss_irq (struct irq_desc *desc) {struct dpu_mdss *dpu_mdss =irq_desc_get_handler_data (desc);@@ -134,7 +160,11 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) {struct dpu_mdss *dpu_mdss =to_dpu_mdss (mdss);struct dss_module_power *mp = &dpu_mdss->mp; -int ret; +int ret, i; +u64 avg_bw = dpu_mdss->num_paths ?MAX_BW / dpu_mdss->num_paths :0 ; + +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_set_bw (dpu_mdss->path[i], avg_bw,kBps_to_icc (MAX_BW )); ret =msm_dss_enable_clk (mp->clk_config, mp->num_clk, true);if (ret)@@ -147,12 +177,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss) {struct dpu_mdss *dpu_mdss =to_dpu_mdss (mdss);struct dss_module_power *mp = &dpu_mdss->mp; -int ret; +int ret, i; ret =msm_dss_enable_clk (mp->clk_config, mp->num_clk, false);if (ret)DPU_ERROR ("clock disable failed, ret:%d\n" , ret); +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_set_bw (dpu_mdss->path[i],0 ,0 ); +return ret; }@@ -163,6 +196,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)struct dpu_mdss *dpu_mdss =to_dpu_mdss (priv->mdss);struct dss_module_power *mp = &dpu_mdss->mp;int irq; +int i;pm_runtime_suspend (dev->dev);pm_runtime_disable (dev->dev);@@ -172,6 +206,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)msm_dss_put_clk (mp->clk_config, mp->num_clk);devm_kfree (&pdev->dev, mp->clk_config); +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_put (dpu_mdss->path[i]); +if (dpu_mdss->mmio)devm_iounmap (&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio =NULL ;@@ -211,6 +248,10 @@ int dpu_mdss_init(struct drm_device *dev) } dpu_mdss->mmio_len =resource_size (res); + ret =dpu_mdss_parse_data_bus_icc_path (dev, dpu_mdss); +if (ret) +return ret; + mp = &dpu_mdss->mp; ret =msm_dss_parse_clock (pdev, mp);if (ret) {@@ -232,14 +273,14 @@ int dpu_mdss_init(struct drm_device *dev)irq_set_chained_handler_and_data (irq, dpu_mdss_irq, dpu_mdss); + priv->mdss = &dpu_mdss->base; +pm_runtime_enable (dev->dev);pm_runtime_get_sync (dev->dev); dpu_mdss->hwversion =readl_relaxed (dpu_mdss->mmio);pm_runtime_put_sync (dev->dev); - priv->mdss = &dpu_mdss->base; -return ret; irq_error:Signature · 1 line
2.20.1
Signature · 5 lines
Sean Paul, Software Engineer, Google / Chromium OS _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 3/5 dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845
In reply to Rob ClarkFrom: Jayant Shekhar <[email protected]>
Add interconnect properties such as interconnect provider specifier , the edge source and destination ports which are required by the interconnect API to configure interconnect path for MDSS.
Changes in v2:
- None
Changes in v3:
- Remove common property definitions (Rob Herring)
Changes in v4:
- Use port macros and change port string names (Georgi Djakov)
Changes in v5-v7:
- None
--- Documentation/devicetree/bindings/display/msm/dpu.txt | 10 ++++++++++ 1 file changed, 10 insertions(+)
Documentation/devicetree/bindings/display/msm/dpu.txt +10 -0
@@ -28,6 +28,11 @@ Required properties: - #address-cells: number of address cells for the MDSS children. Should be 1. - #size-cells: Should be 1. - ranges: parent bus address space is the same as the child bus address space. +- interconnects : interconnect path specifier for MDSS according to + Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be + 2 paths corresponding to 2 AXI ports. +- interconnect-names : MDSS will have 2 port names to differentiate between the + 2 interconnect paths defined with interconnect specifier. Optional properties: - assigned-clocks: list of clock specifiers for clocks needing rate assignment
@@ -86,6 +91,11 @@ Example: interrupt-controller; #interrupt-cells = <1>; + interconnects = <&rsc_hlos MASTER_MDP0 &rsc_hlos SLAVE_EBI1>, + <&rsc_hlos MASTER_MDP1 &rsc_hlos SLAVE_EBI1>; + + interconnect-names = "mdp0-mem", "mdp1-mem"; + iommus = <&apps_iommu 0>; #address-cells = <2>;
Signature · 1 line
2.20.1
PATCH 4/5 drm/msm/dpu: add icc voting in dpu_mdss_init
In reply to Rob ClarkFrom: Abhinav Kumar <[email protected]>
dpu_mdss_destroy() can get called not just from msm_drm_uninit() but also from msm_drm_bind() in case of any failures.
dpu_mdss_destroy() removes the icc voting by calling icc_put. This could accidentally remove the voting done by pm_runtime_enable.
To make the voting balanced add a minimum vote in dpu_mdss_init() to avoid any unclocked access.
This change depends on the following patch which introduces interconnect binding to MDSS driver:
https://patchwork.codeaurora.org/patch/708155/
--- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +14 -4
@@ -49,6 +49,16 @@ static int dpu_mdss_parse_data_bus_icc_path(struct drm_device *dev,return 0 ; } +static void dpu_mdss_icc_request_bw (struct msm_mdss *mdss) +{ +struct dpu_mdss *dpu_mdss =to_dpu_mdss (mdss); +int i; +u64 avg_bw = dpu_mdss->num_paths ?MAX_BW / dpu_mdss->num_paths :0 ; + +for (i =0 ; i < dpu_mdss->num_paths; i++) +icc_set_bw (dpu_mdss->path[i], avg_bw,kBps_to_icc (MAX_BW )); +} +static void dpu_mdss_irq (struct irq_desc *desc) {struct dpu_mdss *dpu_mdss =irq_desc_get_handler_data (desc);
@@ -160,11 +170,9 @@ static int dpu_mdss_enable(struct msm_mdss *mdss) {struct dpu_mdss *dpu_mdss =to_dpu_mdss (mdss);struct dss_module_power *mp = &dpu_mdss->mp; -int ret, i; -u64 avg_bw = dpu_mdss->num_paths ?MAX_BW / dpu_mdss->num_paths :0 ; +int ret; -for (i =0 ; i < dpu_mdss->num_paths; i++) -icc_set_bw (dpu_mdss->path[i], avg_bw,kBps_to_icc (MAX_BW )); +dpu_mdss_icc_request_bw (mdss); ret =msm_dss_enable_clk (mp->clk_config, mp->num_clk, true);if (ret)
@@ -277,6 +285,8 @@ int dpu_mdss_init(struct drm_device *dev)pm_runtime_enable (dev->dev); +dpu_mdss_icc_request_bw (priv->mdss); +pm_runtime_get_sync (dev->dev); dpu_mdss->hwversion =readl_relaxed (dpu_mdss->mmio);pm_runtime_put_sync (dev->dev);
Signature · 1 line
2.20.1
PATCH 5/5 drm/msm/mdp5: Use the interconnect API
In reply to Rob ClarkFrom: Georgi Djakov <[email protected]>
The interconnect API provides an interface for consumer drivers to express their bandwidth needs in the SoC. This data is aggregated and the on-chip interconnect hardware is configured to the most appropriate power/performance profile.
Use the API to configure the interconnects and request bandwidth between DDR and the display hardware (MDP port(s) and rotator downscaler).
v2: update the path names to be consistent with dpu, handle the NULL
path case, updated commit msg from Georgi.
--- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +14 -0
@@ -16,6 +16,7 @@* this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/interconnect.h> #include <linux/of_irq.h> #include "msm_drv.h"
@@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {static int mdp5_dev_probe (struct platform_device *pdev) { +struct icc_path *path0 =of_icc_get (&pdev->dev,"mdp0-mem" ); +struct icc_path *path1 =of_icc_get (&pdev->dev,"mdp1-mem" ); +struct icc_path *path_rot =of_icc_get (&pdev->dev,"rotator-mem" ); + +if (IS_ERR_OR_NULL (path0)) +return PTR_ERR_OR_ZERO (path0); +icc_set_bw (path0,0 ,MBps_to_icc (6400 )); + +if (!IS_ERR_OR_NULL (path1)) +icc_set_bw (path1,0 ,MBps_to_icc (6400 )); +if (!IS_ERR_OR_NULL (path_rot)) +icc_set_bw (path_rot,0 ,MBps_to_icc (6400 )); +DBG ("" );return component_add (&pdev->dev, &mdp5_ops); }
Signature · 6 lines
2.20.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API
Quoted 34 lines · Rob Clark
From: Georgi Djakov <[email protected]>
The interconnect API provides an interface for consumer drivers to express their bandwidth needs in the SoC. This data is aggregated and the on-chip interconnect hardware is configured to the most appropriate power/performance profile.
Use the API to configure the interconnects and request bandwidth between DDR and the display hardware (MDP port(s) and rotator downscaler).
v2: update the path names to be consistent with dpu, handle the NULL
path case, updated commit msg from Georgi.Signed-off-by Georgi Djakov <[email protected]>Signed-off-by Rob Clark <[email protected]>--- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +1
-0@@ -16,6 +16,7 @@* this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/interconnect.h> #include <linux/of_irq.h> #include "msm_drv.h"
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c static const struct component_ops mdp5_ops = { · line 1059
Quoted 9 lines
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +6
-0@@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {static int mdp5_dev_probe (struct platform_device *pdev) { +struct icc_path *path0 =of_icc_get (&pdev->dev,"mdp0-mem" ); +struct icc_path *path1 =of_icc_get (&pdev->dev,"mdp1-mem" ); +struct icc_path *path_rot =of_icc_get (&pdev->dev,"rotator-mem" ); + +if (IS_ERR_OR_NULL (path0)) +return PTR_ERR_OR_ZERO (path0);
Umm, am I misunderstanding something? It seems like of_icc_get() returns NULL if the property doesn't exist. Won't this be backwards incompatible? Existing DTs won't specify the property, and I don't believe the property is supported on all targets. Seems like we'll break things by not calling the below component_add() if the interconnect is not supported, specified, or the interconnect driver is not compiled.
Quoted 13 lines
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +7
-1+icc_set_bw (path0,0 ,MBps_to_icc (6400 )); + +if (!IS_ERR_OR_NULL (path1)) +icc_set_bw (path1,0 ,MBps_to_icc (6400 )); +if (!IS_ERR_OR_NULL (path_rot)) +icc_set_bw (path_rot,0 ,MBps_to_icc (6400 )); +DBG ("" );return component_add (&pdev->dev, &mdp5_ops); } --2.20.1
Quoted 4 lines
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 5/5] drm/msm/mdp5: Use the interconnect API
Quoted 37 lines · Jeffrey Hugo
Quoted 35 lines · Rob Clark
From: Georgi Djakov <[email protected]>
The interconnect API provides an interface for consumer drivers to express their bandwidth needs in the SoC. This data is aggregated and the on-chip interconnect hardware is configured to the most appropriate power/performance profile.
Use the API to configure the interconnects and request bandwidth between DDR and the display hardware (MDP port(s) and rotator downscaler).
v2: update the path names to be consistent with dpu, handle the NULL
path case, updated commit msg from Georgi.Signed-off-by Georgi Djakov <[email protected]>Signed-off-by Rob Clark <[email protected]>--- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +1
-0@@ -16,6 +16,7 @@* this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/interconnect.h> #include <linux/of_irq.h> #include "msm_drv.h" @@ -1050,6 +1051,19 @@ static const struct component_ops mdp5_ops = {
Quoted 16 lines
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c static const struct component_ops mdp5_ops = { · line 1059
Quoted 8 lines
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +6
-0static int mdp5_dev_probe (struct platform_device *pdev) { +struct icc_path *path0 =of_icc_get (&pdev->dev,"mdp0-mem" ); +struct icc_path *path1 =of_icc_get (&pdev->dev,"mdp1-mem" ); +struct icc_path *path_rot =of_icc_get (&pdev->dev,"rotator-mem" ); + +if (IS_ERR_OR_NULL (path0)) +return PTR_ERR_OR_ZERO (path0);Umm, am I misunderstanding something? It seems like
of_icc_get()returns NULL if the property doesn't exist. Won't this be backwards incompatible? Existing DTs won't specify the property, and I don't believe the property is supported on all targets. Seems like we'll break things by not calling the belowcomponent_add()if the interconnect is not supported, specified, or the interconnect driver is not compiled.
hmm, right, I guess I should test this w/out the dts patch.. probably should just revert back to the previous logic..
BR,
-R
Quoted 17 lines
Quoted 17 lines
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +7
-1+icc_set_bw (path0,0 ,MBps_to_icc (6400 )); + +if (!IS_ERR_OR_NULL (path1)) +icc_set_bw (path1,0 ,MBps_to_icc (6400 )); +if (!IS_ERR_OR_NULL (path_rot)) +icc_set_bw (path_rot,0 ,MBps_to_icc (6400 )); +DBG ("" );return component_add (&pdev->dev, &mdp5_ops); } --2.20.1
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
PATCH v3 5/5 drm/msm/mdp5: Use the interconnect API
In reply to Jeffrey HugoFrom: Georgi Djakov <[email protected]>
The interconnect API provides an interface for consumer drivers to express their bandwidth needs in the SoC. This data is aggregated and the on-chip interconnect hardware is configured to the most appropriate power/performance profile.
Use the API to configure the interconnects and request bandwidth between DDR and the display hardware (MDP port(s) and rotator downscaler).
v2: update the path names to be consistent with dpu, handle the NULL
path case, updated commit msg from Georgi.
v3: split out icc setup into it's own function, and rework logic
slightly so no interconnect paths is not fatal.
--- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 38 ++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +38 -0
@@ -16,6 +16,7 @@* this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/interconnect.h> #include <linux/of_irq.h> #include "msm_drv.h"
@@ -1048,9 +1049,46 @@ static const struct component_ops mdp5_ops = { .unbind = mdp5_unbind, }; +static int mdp5_setup_interconnect (struct platform_device *pdev) +{ +struct icc_path *path0 =of_icc_get (&pdev->dev,"mdp0-mem" ); +struct icc_path *path1 =of_icc_get (&pdev->dev,"mdp1-mem" ); +struct icc_path *path_rot =of_icc_get (&pdev->dev,"rotator-mem" ); + +if (IS_ERR (path0)) +return PTR_ERR (path0); + +if (!path0) { +/* no interconnect support is not necessarily a fatal +* condition, the platform may simply not have an +* interconnect driver yet. But warn about it in case +* bootloader didn't setup bus clocks high enough for +* scanout. +*/ +dev_warn (&pdev->dev,"No interconnect support may cause display underflows!\n" ); +return 0 ; + } + +icc_set_bw (path0,0 ,MBps_to_icc (6400 )); + +if (!IS_ERR_OR_NULL (path1)) +icc_set_bw (path1,0 ,MBps_to_icc (6400 )); +if (!IS_ERR_OR_NULL (path_rot)) +icc_set_bw (path_rot,0 ,MBps_to_icc (6400 )); + +return 0 ; +} +static int mdp5_dev_probe (struct platform_device *pdev) { +int ret; +DBG ("" ); + + ret =mdp5_setup_interconnect (pdev); +if (ret) +return ret; +return component_add (&pdev->dev, &mdp5_ops); }
Signature · 6 lines
2.20.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/5 v3] drm/msm/mdp5: Use the interconnect API
Quoted 12 lines · Rob Clark
From: Georgi Djakov <[email protected]>
The interconnect API provides an interface for consumer drivers to express their bandwidth needs in the SoC. This data is aggregated and the on-chip interconnect hardware is configured to the most appropriate power/performance profile.
Use the API to configure the interconnects and request bandwidth between DDR and the display hardware (MDP port(s) and rotator downscaler).
Quoted 7 lines
v2: update the path names to be consistent with dpu, handle the NULL
path case, updated commit msg from Georgi.
v3: split out icc setup into it's own function, and rework logic
slightly so no interconnect paths is not fatal.Signed-off-by Georgi Djakov <[email protected]>Signed-off-by Rob Clark <[email protected]>
Looks good to me.