On 2018-08-31 07:56, Sean Paul wrote:
On Tue, Aug 28, 2018 at 05:39:59PM -0700, Jeykumar Sankaran wrote:
Prep changes for state based resource management.
Moves all the hw block tracking for the crtc to the state object.
Changes in v4...
Signed-off-by: Jeykumar Sankaran jsanka@codeaurora.org
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 60
++++++++++++++++++--------------
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 22 ++++++------ 2 files changed, 44 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e061847..7ab320d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -163,9 +163,9 @@ static void _dpu_crtc_program_lm_output_roi(struct
drm_crtc *crtc)
crtc_state = to_dpu_crtc_state(crtc->state);
lm_horiz_position = 0;
- for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++) {
- for (lm_idx = 0; lm_idx < crtc_state->num_mixers; lm_idx++) { const struct drm_rect *lm_roi =
&crtc_state->lm_bounds[lm_idx];
struct dpu_hw_mixer *hw_lm =dpu_crtc->mixers[lm_idx].hw_lm;
struct dpu_hw_mixer *hw_lm =crtc_state->mixers[lm_idx].hw_lm;
struct dpu_hw_mixer_cfg cfg; if (!lm_roi || !drm_rect_visible(lm_roi))@@ -246,7 +246,7 @@ static void _dpu_crtc_blend_setup_mixer(struct
drm_crtc *crtc,
fb ? fb->modifier : 0); /* blend config update */
for (lm_idx = 0; lm_idx < dpu_crtc->num_mixers; lm_idx++){
for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) { _dpu_crtc_setup_blend_cfg(mixer + lm_idx, pstate, format);@@ -270,7 +270,7 @@ 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 *dpu_crtc_state;
- struct dpu_crtc_state *cstate; struct dpu_crtc_mixer *mixer; struct dpu_hw_ctl *ctl; struct dpu_hw_mixer *lm;
@@ -281,17 +281,17 @@ static void _dpu_crtc_blend_setup(struct drm_crtc
*crtc)
return;dpu_crtc = to_dpu_crtc(crtc);
- dpu_crtc_state = to_dpu_crtc_state(crtc->state);
- mixer = dpu_crtc->mixers;
cstate = to_dpu_crtc_state(crtc->state);
mixer = cstate->mixers;
DPU_DEBUG("%s\n", dpu_crtc->name);
- if (dpu_crtc->num_mixers > CRTC_DUAL_MIXERS) {
DPU_ERROR("invalid number mixers: %d\n",dpu_crtc->num_mixers);
- if (cstate->num_mixers > CRTC_DUAL_MIXERS) {
This is not possible.
DPU_ERROR("invalid number mixers: %d\n",cstate->num_mixers);
return;}
- for (i = 0; i < dpu_crtc->num_mixers; i++) {
- for (i = 0; i < cstate->num_mixers; i++) { if (!mixer[i].hw_lm || !mixer[i].hw_ctl) { DPU_ERROR("invalid lm or ctl assigned to
mixer\n");
return;@@ -308,7 +308,7 @@ static void _dpu_crtc_blend_setup(struct drm_crtc
*crtc)
_dpu_crtc_blend_setup_mixer(crtc, dpu_crtc, mixer);
- for (i = 0; i < dpu_crtc->num_mixers; i++) {
- for (i = 0; i < cstate->num_mixers; i++) { ctl = mixer[i].hw_ctl; lm = mixer[i].hw_lm;
@@ -530,7 +530,7 @@ static void _dpu_crtc_setup_mixer_for_encoder( struct drm_crtc *crtc, struct drm_encoder *enc) {
- struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
- struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); struct dpu_rm *rm = &dpu_kms->rm; struct dpu_crtc_mixer *mixer;
@@ -542,8 +542,8 @@ static void _dpu_crtc_setup_mixer_for_encoder( dpu_rm_init_hw_iter(&ctl_iter, enc->base.id, DPU_HW_BLK_CTL);
/* Set up all the mixers and ctls reserved by this encoder */
- for (i = dpu_crtc->num_mixers; i < ARRAY_SIZE(dpu_crtc->mixers);
i++) {
mixer = &dpu_crtc->mixers[i];
- for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++)
{
mixer = &cstate->mixers[i];if (!dpu_rm_get_hw(rm, &lm_iter)) break;
@@ -568,7 +568,7 @@ static void _dpu_crtc_setup_mixer_for_encoder(
mixer->encoder = enc;
dpu_crtc->num_mixers++;
cstate->num_mixers++;@@ -579,11 +579,11 @@ static void _dpu_crtc_setup_mixer_for_encoder( static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc) { struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
- struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state); struct drm_encoder *enc;
- dpu_crtc->num_mixers = 0;
- dpu_crtc->mixers_swapped = false;
- memset(dpu_crtc->mixers, 0, sizeof(dpu_crtc->mixers));
- cstate->num_mixers = 0;
- memset(cstate->mixers, 0, sizeof(cstate->mixers));
Why is this necessary?
mutex_lock(&dpu_crtc->crtc_lock); /* Check for mixers on all encoders attached to this crtc */ @@ -618,7 +618,7 @@ static void _dpu_crtc_setup_lm_bounds(struct
drm_crtc *crtc,
crtc_split_width = _dpu_crtc_get_mixer_width(dpu_crtc, cstate, adj_mode);
- for (i = 0; i < dpu_crtc->num_mixers; i++) {
- for (i = 0; i < cstate->num_mixers; i++) { struct drm_rect *r = &cstate->lm_bounds[i]; r->x1 = crtc_split_width * i; r->y1 = 0;
@@ -635,6 +635,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
*crtc,
struct drm_crtc_state *old_state){ struct dpu_crtc *dpu_crtc;
- struct dpu_crtc_state *cstate; struct drm_encoder *encoder; struct drm_device *dev; unsigned long flags;
@@ -654,10 +655,11 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
*crtc,
DPU_DEBUG("crtc%d\n", crtc->base.id);
dpu_crtc = to_dpu_crtc(crtc);
- cstate = to_dpu_crtc_state(crtc->state); dev = crtc->dev; smmu_state = &dpu_crtc->smmu_state;
- if (!dpu_crtc->num_mixers) {
- if (!cstate->num_mixers) { _dpu_crtc_setup_mixers(crtc); _dpu_crtc_setup_lm_bounds(crtc, crtc->state); }
@@ -684,7 +686,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
*crtc,
* it means we are trying to flush a CRTC whose state is disabled: * nothing else needs to be done. */
- if (unlikely(!dpu_crtc->num_mixers))
if (unlikely(!cstate->num_mixers)) return;
_dpu_crtc_blend_setup(crtc);
@@ -748,7 +750,7 @@ static void dpu_crtc_atomic_flush(struct drm_crtc
*crtc,
* it means we are trying to flush a CRTC whose state is disabled: * nothing else needs to be done. */
- if (unlikely(!dpu_crtc->num_mixers))
if (unlikely(!cstate->num_mixers)) return;
/*
@@ -863,7 +865,7 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) * it means we are trying to start a CRTC whose state is disabled: * nothing else needs to be done. */
- if (unlikely(!dpu_crtc->num_mixers))
if (unlikely(!cstate->num_mixers)) return;
DPU_ATRACE_BEGIN("crtc_commit");
@@ -1097,12 +1099,14 @@ static void dpu_crtc_handle_power_event(u32
event_type, void *arg)
struct drm_crtc *crtc = arg; struct dpu_crtc *dpu_crtc; struct drm_encoder *encoder;
struct dpu_crtc_state *cstate;
if (!crtc) { DPU_ERROR("invalid crtc\n"); return; } dpu_crtc = to_dpu_crtc(crtc);
cstate = to_dpu_crtc_state(dpu_crtc->base.state);
mutex_lock(&dpu_crtc->crtc_lock);
@@ -1197,9 +1201,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
dpu_power_handle_unregister_event(dpu_crtc->phandle, dpu_crtc->power_event);
- memset(dpu_crtc->mixers, 0, sizeof(dpu_crtc->mixers));
- dpu_crtc->num_mixers = 0;
- dpu_crtc->mixers_swapped = false;
- memset(cstate->mixers, 0, sizeof(cstate->mixers));
Same question here, why is this necessary?
cstate->num_mixers = 0;
/* disable clk & bw control until clk & bw properties are set */ cstate->bw_control = false;
@@ -1547,6 +1550,8 @@ static int _dpu_debugfs_status_show(struct
seq_file *s, void *data)
dpu_crtc = s->private; crtc = &dpu_crtc->base;
- drm_modeset_lock(&crtc->mutex, NULL);
When I suggested this, I was hoping you'd put it in a separate patch. Additionally, you'll probably want modeset_lock_all instead.
crtc state retrieval is introduced in this patch. So I thought it would make sense to add the locking as well a part of this patch.
cstate = to_dpu_crtc_state(crtc->state);
mutex_lock(&dpu_crtc->crtc_lock); @@ -1558,8 +1563,8 @@ static int _dpu_debugfs_status_show(struct
seq_file *s, void *data)
seq_puts(s, "\n");
- for (i = 0; i < dpu_crtc->num_mixers; ++i) {
m = &dpu_crtc->mixers[i];
- for (i = 0; i < cstate->num_mixers; ++i) {
m = &cstate->mixers[i];@@ -1639,6 +1644,7 @@ static int _dpu_debugfs_status_show(struct
seq_file *s, void *data)
seq_printf(s, "vblank_enable:%d\n", dpu_crtc->vblank_requested);
mutex_unlock(&dpu_crtc->crtc_lock);
drm_modeset_unlock(&crtc->mutex);
return 0;
} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 5e4dc5c..7aa772f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h @@ -121,11 +121,6 @@ struct dpu_crtc_frame_event {
- struct dpu_crtc - virtualized CRTC data structure
- @base : Base drm crtc structure
- @name : ASCII description of this crtc
- @num_ctls : Number of ctl paths in use
- @num_mixers : Number of mixers in use
- @mixers_swapped: Whether the mixers have been swapped for
left/right
update
especially in the case of DSC Merge.
- @mixers : List of active mixers
- @event : Pointer to last received drm vblank event. If
there
is a
pending vblank event, this will be non-null.- @vsync_count : Running count of received vsync events
@@ -164,12 +159,6 @@ struct dpu_crtc { struct drm_crtc base; char name[DPU_CRTC_NAME_SIZE];
- /* HW Resources reserved for the crtc */
- u32 num_ctls;
- u32 num_mixers;
- bool mixers_swapped;
- struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
- struct drm_pending_vblank_event *event; u32 vsync_count;
@@ -221,6 +210,10 @@ struct dpu_crtc {
- @property_values: Current crtc property values
- @input_fence_timeout_ns : Cached input fence timeout, in ns
- @new_perf: new performance state being requested
- @num_mixers : Number of mixers in use
- @mixers : List of active mixers
- @num_ctls : Number of ctl paths in use
*/
- @hw_ctls : List of active ctl paths
struct dpu_crtc_state { struct drm_crtc_state base; @@ -232,6 +225,13 @@ struct dpu_crtc_state { uint64_t input_fence_timeout_ns;
struct dpu_core_perf_params new_perf;
- /* HW Resources reserved for the crtc */
- u32 num_mixers;
- struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
- u32 num_ctls;
- struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
};
#define to_dpu_crtc_state(x) \
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project