| Message ID | 1535503203-22054-13-git-send-email-jsanka@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clean up DPU for RM refactor | expand |
On Tue, Aug 28, 2018 at 05:40:01PM -0700, Jeykumar Sankaran wrote: > Strip down the support for topology enums. It > can be replaced with simple hw count checks. > Changes in v4: ... > Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 -- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 1 - > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 9 ++++-- > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 36 ++++++---------------- > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 24 --------------- > 6 files changed, 19 insertions(+), 61 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index dbf669e..56ef349 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1013,7 +1013,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, > struct drm_connector *conn = NULL, *conn_iter; > struct dpu_rm_hw_iter pp_iter, ctl_iter; > struct msm_display_topology topology; > - enum dpu_rm_topology_name topology_name; > struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL }; > int i = 0, ret; > > @@ -1069,7 +1068,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, > hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw; > } > > - topology_name = dpu_rm_get_topology_name(topology); > for (i = 0; i < dpu_enc->num_phys_encs; i++) { > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > > @@ -1089,7 +1087,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, > phys->hw_ctl = hw_ctl[i]; > > phys->connector = conn->state->connector; > - phys->topology_name = topology_name; > if (phys->ops.mode_set) > phys->ops.mode_set(phys, mode, adj_mode); > } > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > index 60f809f..c5600e6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > @@ -35,7 +35,6 @@ > * @needs_cdm: Encoder requests a CDM based on pixel format conversion needs > * @display_num_of_h_tiles: Number of horizontal tiles in case of split > * interface > - * @topology: Topology of the display > */ > struct dpu_encoder_hw_resources { > enum dpu_intf_mode intfs[INTF_MAX]; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > index b3917e0..b5fc65c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > @@ -24,6 +24,7 @@ > #include "dpu_hw_top.h" > #include "dpu_hw_cdm.h" > #include "dpu_encoder.h" > +#include "dpu_crtc.h" > > #define DPU_ENCODER_NAME_MAX 16 > > @@ -213,7 +214,6 @@ struct dpu_encoder_irq { > * @split_role: Role to play in a split-panel configuration > * @intf_mode: Interface mode > * @intf_idx: Interface index on dpu hardware > - * @topology_name: topology selected for the display > * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes > * @enable_state: Enable state tracking > * @vblank_refcount: Reference count of vblank request > @@ -243,7 +243,6 @@ struct dpu_encoder_phys { > enum dpu_enc_split_role split_role; > enum dpu_intf_mode intf_mode; > enum dpu_intf intf_idx; > - enum dpu_rm_topology_name topology_name; > spinlock_t *enc_spinlock; > enum dpu_enc_enable_state enable_state; > atomic_t vblank_refcount; > @@ -361,11 +360,15 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( > static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode( > struct dpu_encoder_phys *phys_enc) > { > + struct dpu_crtc_state *dpu_cstate; > + > if (!phys_enc || phys_enc->enable_state == DPU_ENC_DISABLING) > return BLEND_3D_NONE; > > + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); > + > if (phys_enc->split_role == ENC_ROLE_SOLO && > - phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE) > + (dpu_cstate->num_mixers == 2)) I guess this should be CRTC_DUAL_MIXERS? Perhaps you should add a helper called dpu_crtc_state_is_stereo() instead of littering these checks everywhere. > return BLEND_3D_H_ROW_INT; > > return BLEND_3D_NONE; > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > index 6de13f4..1643e2f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > @@ -355,13 +355,14 @@ static void dpu_encoder_phys_vid_underrun_irq(void *arg, int irq_idx) > > static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys *phys_enc) > { > + struct dpu_crtc_state *dpu_cstate; > + > if (!phys_enc) > return false; > > - if (phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE) > - return true; > + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); > > - return false; > + return (dpu_cstate->num_ctls > 1); Unnecessary parens > } > > static bool dpu_encoder_phys_vid_needs_single_flush( > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > index 13c0a36..fbd489f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > @@ -33,7 +33,6 @@ > (t).num_intf == (r).num_intf) > > struct dpu_rm_topology_def { > - enum dpu_rm_topology_name top_name; > int num_lm; > int num_comp_enc; > int num_intf; > @@ -42,10 +41,10 @@ struct dpu_rm_topology_def { > }; > > static const struct dpu_rm_topology_def g_top_table[] = { > - { DPU_RM_TOPOLOGY_NONE, 0, 0, 0, 0, false }, > - { DPU_RM_TOPOLOGY_SINGLEPIPE, 1, 0, 1, 1, false }, > - { DPU_RM_TOPOLOGY_DUALPIPE, 2, 0, 2, 2, true }, > - { DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, 2, 0, 1, 1, false }, > + { 0, 0, 0, 0, false }, > + { 1, 0, 1, 1, false }, > + { 2, 0, 2, 2, true }, > + { 2, 0, 1, 1, false }, You don't need this. The data it provides can be deduced already: num_ctl == num_intf needs_split_display == (num_intf > 1) > }; > > /** > @@ -72,13 +71,11 @@ struct dpu_rm_requirements { > * @enc_id: Reservations are tracked by Encoder DRM object ID. > * CRTCs may be connected to multiple Encoders. > * An encoder or connector id identifies the display path. > - * @topology DRM<->HW topology use case > */ > struct dpu_rm_rsvp { > struct list_head list; > uint32_t seq; > uint32_t enc_id; > - enum dpu_rm_topology_name topology; > }; > > /** > @@ -122,8 +119,8 @@ static void _dpu_rm_print_rsvps( > DPU_DEBUG("%d\n", stage); > > list_for_each_entry(rsvp, &rm->rsvps, list) { > - DRM_DEBUG_KMS("%d rsvp[s%ue%u] topology %d\n", stage, rsvp->seq, > - rsvp->enc_id, rsvp->topology); > + DRM_DEBUG_KMS("%d rsvp[s%ue%u]\n", stage, rsvp->seq, > + rsvp->enc_id); > } > > for (type = 0; type < DPU_HW_BLK_MAX; type++) { > @@ -146,18 +143,6 @@ struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm) > return rm->hw_mdp; > } > > -enum dpu_rm_topology_name > -dpu_rm_get_topology_name(struct msm_display_topology topology) > -{ > - int i; > - > - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) > - if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], topology)) > - return g_top_table[i].top_name; > - > - return DPU_RM_TOPOLOGY_NONE; > -} > - > void dpu_rm_init_hw_iter( > struct dpu_rm_hw_iter *iter, > uint32_t enc_id, > @@ -760,7 +745,6 @@ static int _dpu_rm_make_next_rsvp( > /* Create reservation info, tag reserved blocks with it as we go */ > rsvp->seq = ++rm->rsvp_next_seq; > rsvp->enc_id = enc->base.id; > - rsvp->topology = reqs->topology->top_name; > list_add_tail(&rsvp->list, &rm->rsvps); > > ret = _dpu_rm_reserve_lms(rm, rsvp, reqs); > @@ -807,7 +791,7 @@ static int _dpu_rm_populate_requirements( > > dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state); > > - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) { > + for (i = 0; i < ARRAY_SIZE(g_top_table); i++) { > if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], > req_topology)) { > reqs->topology = &g_top_table[i]; Since you can get rid of g_top_table, you can get rid of reqs->topology too. > @@ -831,9 +815,8 @@ static int _dpu_rm_populate_requirements( > > DRM_DEBUG_KMS("top_ctrl: 0x%llX num_h_tiles: %d\n", reqs->top_ctrl, > reqs->hw_res.display_num_of_h_tiles); > - DRM_DEBUG_KMS("num_lm: %d num_ctl: %d topology: %d split_display: %d\n", > + DRM_DEBUG_KMS("num_lm: %d num_ctl: %d split_display: %d\n", > reqs->topology->num_lm, reqs->topology->num_ctl, > - reqs->topology->top_name, > reqs->topology->needs_split_display); > > return 0; > @@ -969,8 +952,7 @@ static int _dpu_rm_commit_rsvp( > } > > if (!ret) > - DRM_DEBUG_KMS("rsrv enc %d topology %d\n", rsvp->enc_id, > - rsvp->topology); > + DRM_DEBUG_KMS("rsrv enc %d\n", rsvp->enc_id); This debug message doesn't provide any value any longer. > > return ret; > } > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > index ffd1841..6fd9e7b 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > @@ -21,21 +21,6 @@ > #include "dpu_hw_top.h" > > /** > - * enum dpu_rm_topology_name - HW resource use case in use by connector > - * @DPU_RM_TOPOLOGY_NONE: No topology in use currently > - * @DPU_RM_TOPOLOGY_SINGLEPIPE: 1 LM, 1 PP, 1 INTF/WB > - * @DPU_RM_TOPOLOGY_DUALPIPE: 2 LM, 2 PP, 2 INTF/WB > - * @DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE: 2 LM, 2 PP, 3DMux, 1 INTF/WB > - */ > -enum dpu_rm_topology_name { > - DPU_RM_TOPOLOGY_NONE = 0, > - DPU_RM_TOPOLOGY_SINGLEPIPE, > - DPU_RM_TOPOLOGY_DUALPIPE, > - DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, > - DPU_RM_TOPOLOGY_MAX, > -}; > - > -/** > * enum dpu_rm_topology_control - HW resource use case in use by connector > * @DPU_RM_TOPCTL_RESERVE_LOCK: If set, in AtomicTest phase, after a successful > * test, reserve the resources for this display. > @@ -187,13 +172,4 @@ void dpu_rm_init_hw_iter( > */ > int dpu_rm_check_property_topctl(uint64_t val); > > -/** > - * dpu_rm_get_topology_name - returns the name of the the given topology > - * definition > - * @topology: topology definition > - * @Return: name of the topology > - */ > -enum dpu_rm_topology_name > -dpu_rm_get_topology_name(struct msm_display_topology topology); > - > #endif /* __DPU_RM_H__ */ > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
On 2018-08-31 09:08, Sean Paul wrote: > On Tue, Aug 28, 2018 at 05:40:01PM -0700, Jeykumar Sankaran wrote: >> Strip down the support for topology enums. It >> can be replaced with simple hw count checks. >> > > Changes in v4: > ... > >> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 -- >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 1 - >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 9 ++++-- >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++-- >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 36 > ++++++---------------- >> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 24 >> --------------- >> 6 files changed, 19 insertions(+), 61 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> index dbf669e..56ef349 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c >> @@ -1013,7 +1013,6 @@ static void dpu_encoder_virt_mode_set(struct > drm_encoder *drm_enc, >> struct drm_connector *conn = NULL, *conn_iter; >> struct dpu_rm_hw_iter pp_iter, ctl_iter; >> struct msm_display_topology topology; >> - enum dpu_rm_topology_name topology_name; >> struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL }; >> int i = 0, ret; >> >> @@ -1069,7 +1068,6 @@ static void dpu_encoder_virt_mode_set(struct > drm_encoder *drm_enc, >> hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw; >> } >> >> - topology_name = dpu_rm_get_topology_name(topology); >> for (i = 0; i < dpu_enc->num_phys_encs; i++) { >> struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; >> >> @@ -1089,7 +1087,6 @@ static void dpu_encoder_virt_mode_set(struct > drm_encoder *drm_enc, >> phys->hw_ctl = hw_ctl[i]; >> >> phys->connector = conn->state->connector; >> - phys->topology_name = topology_name; >> if (phys->ops.mode_set) >> phys->ops.mode_set(phys, mode, adj_mode); >> } >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> index 60f809f..c5600e6 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h >> @@ -35,7 +35,6 @@ >> * @needs_cdm: Encoder requests a CDM based on pixel format > conversion needs >> * @display_num_of_h_tiles: Number of horizontal tiles in case of >> split >> * interface >> - * @topology: Topology of the display >> */ >> struct dpu_encoder_hw_resources { >> enum dpu_intf_mode intfs[INTF_MAX]; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> index b3917e0..b5fc65c 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h >> @@ -24,6 +24,7 @@ >> #include "dpu_hw_top.h" >> #include "dpu_hw_cdm.h" >> #include "dpu_encoder.h" >> +#include "dpu_crtc.h" >> >> #define DPU_ENCODER_NAME_MAX 16 >> >> @@ -213,7 +214,6 @@ struct dpu_encoder_irq { >> * @split_role: Role to play in a split-panel > configuration >> * @intf_mode: Interface mode >> * @intf_idx: Interface index on dpu hardware >> - * @topology_name: topology selected for the display >> * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes >> * @enable_state: Enable state tracking >> * @vblank_refcount: Reference count of vblank request >> @@ -243,7 +243,6 @@ struct dpu_encoder_phys { >> enum dpu_enc_split_role split_role; >> enum dpu_intf_mode intf_mode; >> enum dpu_intf intf_idx; >> - enum dpu_rm_topology_name topology_name; >> spinlock_t *enc_spinlock; >> enum dpu_enc_enable_state enable_state; >> atomic_t vblank_refcount; >> @@ -361,11 +360,15 @@ struct dpu_encoder_phys > *dpu_encoder_phys_cmd_init( >> static inline enum dpu_3d_blend_mode > dpu_encoder_helper_get_3d_blend_mode( >> struct dpu_encoder_phys *phys_enc) >> { >> + struct dpu_crtc_state *dpu_cstate; >> + >> if (!phys_enc || phys_enc->enable_state == DPU_ENC_DISABLING) >> return BLEND_3D_NONE; >> >> + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); >> + >> if (phys_enc->split_role == ENC_ROLE_SOLO && >> - phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE) >> + (dpu_cstate->num_mixers == 2)) > > I guess this should be CRTC_DUAL_MIXERS? Perhaps you should add a > helper > called > dpu_crtc_state_is_stereo() instead of littering these checks > everywhere. > >> return BLEND_3D_H_ROW_INT; >> >> return BLEND_3D_NONE; >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c >> index 6de13f4..1643e2f 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c >> @@ -355,13 +355,14 @@ static void >> dpu_encoder_phys_vid_underrun_irq(void > *arg, int irq_idx) >> >> static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys > *phys_enc) >> { >> + struct dpu_crtc_state *dpu_cstate; >> + >> if (!phys_enc) >> return false; >> >> - if (phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE) >> - return true; >> + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); >> >> - return false; >> + return (dpu_cstate->num_ctls > 1); > > Unnecessary parens > >> } >> >> static bool dpu_encoder_phys_vid_needs_single_flush( >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c >> index 13c0a36..fbd489f 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c >> @@ -33,7 +33,6 @@ >> (t).num_intf == (r).num_intf) >> >> struct dpu_rm_topology_def { >> - enum dpu_rm_topology_name top_name; >> int num_lm; >> int num_comp_enc; >> int num_intf; >> @@ -42,10 +41,10 @@ struct dpu_rm_topology_def { >> }; >> >> static const struct dpu_rm_topology_def g_top_table[] = { >> - { DPU_RM_TOPOLOGY_NONE, 0, 0, 0, 0, false }, >> - { DPU_RM_TOPOLOGY_SINGLEPIPE, 1, 0, 1, 1, false }, >> - { DPU_RM_TOPOLOGY_DUALPIPE, 2, 0, 2, 2, true }, >> - { DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, 2, 0, 1, 1, false }, >> + { 0, 0, 0, 0, false }, >> + { 1, 0, 1, 1, false }, >> + { 2, 0, 2, 2, true }, >> + { 2, 0, 1, 1, false }, > > You don't need this. The data it provides can be deduced already: > num_ctl == num_intf > needs_split_display == (num_intf > 1) > >> }; >> >> /** >> @@ -72,13 +71,11 @@ struct dpu_rm_requirements { >> * @enc_id: Reservations are tracked by Encoder DRM object ID. >> * CRTCs may be connected to multiple Encoders. >> * An encoder or connector id identifies the display path. >> - * @topology DRM<->HW topology use case >> */ >> struct dpu_rm_rsvp { >> struct list_head list; >> uint32_t seq; >> uint32_t enc_id; >> - enum dpu_rm_topology_name topology; >> }; >> >> /** >> @@ -122,8 +119,8 @@ static void _dpu_rm_print_rsvps( >> DPU_DEBUG("%d\n", stage); >> >> list_for_each_entry(rsvp, &rm->rsvps, list) { >> - DRM_DEBUG_KMS("%d rsvp[s%ue%u] topology %d\n", stage, > rsvp->seq, >> - rsvp->enc_id, rsvp->topology); >> + DRM_DEBUG_KMS("%d rsvp[s%ue%u]\n", stage, rsvp->seq, >> + rsvp->enc_id); >> } >> >> for (type = 0; type < DPU_HW_BLK_MAX; type++) { >> @@ -146,18 +143,6 @@ struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm > *rm) >> return rm->hw_mdp; >> } >> >> -enum dpu_rm_topology_name >> -dpu_rm_get_topology_name(struct msm_display_topology topology) >> -{ >> - int i; >> - >> - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) >> - if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], topology)) >> - return g_top_table[i].top_name; >> - >> - return DPU_RM_TOPOLOGY_NONE; >> -} >> - >> void dpu_rm_init_hw_iter( >> struct dpu_rm_hw_iter *iter, >> uint32_t enc_id, >> @@ -760,7 +745,6 @@ static int _dpu_rm_make_next_rsvp( >> /* Create reservation info, tag reserved blocks with it as we go > */ >> rsvp->seq = ++rm->rsvp_next_seq; >> rsvp->enc_id = enc->base.id; >> - rsvp->topology = reqs->topology->top_name; >> list_add_tail(&rsvp->list, &rm->rsvps); >> >> ret = _dpu_rm_reserve_lms(rm, rsvp, reqs); >> @@ -807,7 +791,7 @@ static int _dpu_rm_populate_requirements( >> >> dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state); >> >> - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) { >> + for (i = 0; i < ARRAY_SIZE(g_top_table); i++) { >> if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], >> req_topology)) { >> reqs->topology = &g_top_table[i]; > > Since you can get rid of g_top_table, you can get rid of reqs->topology > too. > DPU RM is using reqs->topology (ie. struct dpu_rm_topology_def) to represent the hw_block requirements for a display. Removing the structure might require changes in rest of RM alloc functions. If you can hold off on this, I am planning to address your comment in the RM refactor series. Jeykumar S. >> @@ -831,9 +815,8 @@ static int _dpu_rm_populate_requirements( >> >> DRM_DEBUG_KMS("top_ctrl: 0x%llX num_h_tiles: %d\n", > reqs->top_ctrl, >> reqs->hw_res.display_num_of_h_tiles); >> - DRM_DEBUG_KMS("num_lm: %d num_ctl: %d topology: %d split_display: > %d\n", >> + DRM_DEBUG_KMS("num_lm: %d num_ctl: %d split_display: %d\n", >> reqs->topology->num_lm, reqs->topology->num_ctl, >> - reqs->topology->top_name, >> reqs->topology->needs_split_display); >> >> return 0; >> @@ -969,8 +952,7 @@ static int _dpu_rm_commit_rsvp( >> } >> >> if (!ret) >> - DRM_DEBUG_KMS("rsrv enc %d topology %d\n", rsvp->enc_id, >> - rsvp->topology); >> + DRM_DEBUG_KMS("rsrv enc %d\n", rsvp->enc_id); > > This debug message doesn't provide any value any longer. > >> >> return ret; >> } >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h >> index ffd1841..6fd9e7b 100644 >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h >> @@ -21,21 +21,6 @@ >> #include "dpu_hw_top.h" >> >> /** >> - * enum dpu_rm_topology_name - HW resource use case in use by >> connector >> - * @DPU_RM_TOPOLOGY_NONE: No topology in use >> currently >> - * @DPU_RM_TOPOLOGY_SINGLEPIPE: 1 LM, 1 PP, 1 INTF/WB >> - * @DPU_RM_TOPOLOGY_DUALPIPE: 2 LM, 2 PP, 2 INTF/WB >> - * @DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE: 2 LM, 2 PP, 3DMux, 1 >> INTF/WB >> - */ >> -enum dpu_rm_topology_name { >> - DPU_RM_TOPOLOGY_NONE = 0, >> - DPU_RM_TOPOLOGY_SINGLEPIPE, >> - DPU_RM_TOPOLOGY_DUALPIPE, >> - DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, >> - DPU_RM_TOPOLOGY_MAX, >> -}; >> - >> -/** >> * enum dpu_rm_topology_control - HW resource use case in use by > connector >> * @DPU_RM_TOPCTL_RESERVE_LOCK: If set, in AtomicTest phase, after a > successful >> * test, reserve the resources for this > display. >> @@ -187,13 +172,4 @@ void dpu_rm_init_hw_iter( >> */ >> int dpu_rm_check_property_topctl(uint64_t val); >> >> -/** >> - * dpu_rm_get_topology_name - returns the name of the the given > topology >> - * definition >> - * @topology: topology definition >> - * @Return: name of the topology >> - */ >> -enum dpu_rm_topology_name >> -dpu_rm_get_topology_name(struct msm_display_topology topology); >> - >> #endif /* __DPU_RM_H__ */ >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum, >> a Linux Foundation Collaborative Project >>
On Tue, Sep 04, 2018 at 04:03:25PM -0700, Jeykumar Sankaran wrote: > On 2018-08-31 09:08, Sean Paul wrote: > > On Tue, Aug 28, 2018 at 05:40:01PM -0700, Jeykumar Sankaran wrote: > > > Strip down the support for topology enums. It > > > can be replaced with simple hw count checks. > > > > > > > Changes in v4: > > ... > > > > > Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 -- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 1 - > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 9 ++++-- > > > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++-- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 36 > > ++++++---------------- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 24 > > > --------------- > > > 6 files changed, 19 insertions(+), 61 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > index dbf669e..56ef349 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > > > @@ -1013,7 +1013,6 @@ static void dpu_encoder_virt_mode_set(struct > > drm_encoder *drm_enc, > > > struct drm_connector *conn = NULL, *conn_iter; > > > struct dpu_rm_hw_iter pp_iter, ctl_iter; > > > struct msm_display_topology topology; > > > - enum dpu_rm_topology_name topology_name; > > > struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL }; > > > int i = 0, ret; > > > > > > @@ -1069,7 +1068,6 @@ static void dpu_encoder_virt_mode_set(struct > > drm_encoder *drm_enc, > > > hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw; > > > } > > > > > > - topology_name = dpu_rm_get_topology_name(topology); > > > for (i = 0; i < dpu_enc->num_phys_encs; i++) { > > > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; > > > > > > @@ -1089,7 +1087,6 @@ static void dpu_encoder_virt_mode_set(struct > > drm_encoder *drm_enc, > > > phys->hw_ctl = hw_ctl[i]; > > > > > > phys->connector = conn->state->connector; > > > - phys->topology_name = topology_name; > > > if (phys->ops.mode_set) > > > phys->ops.mode_set(phys, mode, adj_mode); > > > } > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > > > index 60f809f..c5600e6 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h > > > @@ -35,7 +35,6 @@ > > > * @needs_cdm: Encoder requests a CDM based on pixel format > > conversion needs > > > * @display_num_of_h_tiles: Number of horizontal tiles in case of > > > split > > > * interface > > > - * @topology: Topology of the display > > > */ > > > struct dpu_encoder_hw_resources { > > > enum dpu_intf_mode intfs[INTF_MAX]; > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > > > index b3917e0..b5fc65c 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h > > > @@ -24,6 +24,7 @@ > > > #include "dpu_hw_top.h" > > > #include "dpu_hw_cdm.h" > > > #include "dpu_encoder.h" > > > +#include "dpu_crtc.h" > > > > > > #define DPU_ENCODER_NAME_MAX 16 > > > > > > @@ -213,7 +214,6 @@ struct dpu_encoder_irq { > > > * @split_role: Role to play in a split-panel > > configuration > > > * @intf_mode: Interface mode > > > * @intf_idx: Interface index on dpu hardware > > > - * @topology_name: topology selected for the display > > > * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes > > > * @enable_state: Enable state tracking > > > * @vblank_refcount: Reference count of vblank request > > > @@ -243,7 +243,6 @@ struct dpu_encoder_phys { > > > enum dpu_enc_split_role split_role; > > > enum dpu_intf_mode intf_mode; > > > enum dpu_intf intf_idx; > > > - enum dpu_rm_topology_name topology_name; > > > spinlock_t *enc_spinlock; > > > enum dpu_enc_enable_state enable_state; > > > atomic_t vblank_refcount; > > > @@ -361,11 +360,15 @@ struct dpu_encoder_phys > > *dpu_encoder_phys_cmd_init( > > > static inline enum dpu_3d_blend_mode > > dpu_encoder_helper_get_3d_blend_mode( > > > struct dpu_encoder_phys *phys_enc) > > > { > > > + struct dpu_crtc_state *dpu_cstate; > > > + > > > if (!phys_enc || phys_enc->enable_state == DPU_ENC_DISABLING) > > > return BLEND_3D_NONE; > > > > > > + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); > > > + > > > if (phys_enc->split_role == ENC_ROLE_SOLO && > > > - phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE) > > > + (dpu_cstate->num_mixers == 2)) > > > > I guess this should be CRTC_DUAL_MIXERS? Perhaps you should add a helper > > called > > dpu_crtc_state_is_stereo() instead of littering these checks everywhere. > > > > > return BLEND_3D_H_ROW_INT; > > > > > > return BLEND_3D_NONE; > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > index 6de13f4..1643e2f 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > > @@ -355,13 +355,14 @@ static void > > > dpu_encoder_phys_vid_underrun_irq(void > > *arg, int irq_idx) > > > > > > static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys > > *phys_enc) > > > { > > > + struct dpu_crtc_state *dpu_cstate; > > > + > > > if (!phys_enc) > > > return false; > > > > > > - if (phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE) > > > - return true; > > > + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); > > > > > > - return false; > > > + return (dpu_cstate->num_ctls > 1); > > > > Unnecessary parens > > > > > } > > > > > > static bool dpu_encoder_phys_vid_needs_single_flush( > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > > > index 13c0a36..fbd489f 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c > > > @@ -33,7 +33,6 @@ > > > (t).num_intf == (r).num_intf) > > > > > > struct dpu_rm_topology_def { > > > - enum dpu_rm_topology_name top_name; > > > int num_lm; > > > int num_comp_enc; > > > int num_intf; > > > @@ -42,10 +41,10 @@ struct dpu_rm_topology_def { > > > }; > > > > > > static const struct dpu_rm_topology_def g_top_table[] = { > > > - { DPU_RM_TOPOLOGY_NONE, 0, 0, 0, 0, false }, > > > - { DPU_RM_TOPOLOGY_SINGLEPIPE, 1, 0, 1, 1, false }, > > > - { DPU_RM_TOPOLOGY_DUALPIPE, 2, 0, 2, 2, true }, > > > - { DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, 2, 0, 1, 1, false }, > > > + { 0, 0, 0, 0, false }, > > > + { 1, 0, 1, 1, false }, > > > + { 2, 0, 2, 2, true }, > > > + { 2, 0, 1, 1, false }, > > > > You don't need this. The data it provides can be deduced already: > > num_ctl == num_intf > > needs_split_display == (num_intf > 1) > > > > > }; > > > > > > /** > > > @@ -72,13 +71,11 @@ struct dpu_rm_requirements { > > > * @enc_id: Reservations are tracked by Encoder DRM object ID. > > > * CRTCs may be connected to multiple Encoders. > > > * An encoder or connector id identifies the display path. > > > - * @topology DRM<->HW topology use case > > > */ > > > struct dpu_rm_rsvp { > > > struct list_head list; > > > uint32_t seq; > > > uint32_t enc_id; > > > - enum dpu_rm_topology_name topology; > > > }; > > > > > > /** > > > @@ -122,8 +119,8 @@ static void _dpu_rm_print_rsvps( > > > DPU_DEBUG("%d\n", stage); > > > > > > list_for_each_entry(rsvp, &rm->rsvps, list) { > > > - DRM_DEBUG_KMS("%d rsvp[s%ue%u] topology %d\n", stage, > > rsvp->seq, > > > - rsvp->enc_id, rsvp->topology); > > > + DRM_DEBUG_KMS("%d rsvp[s%ue%u]\n", stage, rsvp->seq, > > > + rsvp->enc_id); > > > } > > > > > > for (type = 0; type < DPU_HW_BLK_MAX; type++) { > > > @@ -146,18 +143,6 @@ struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm > > *rm) > > > return rm->hw_mdp; > > > } > > > > > > -enum dpu_rm_topology_name > > > -dpu_rm_get_topology_name(struct msm_display_topology topology) > > > -{ > > > - int i; > > > - > > > - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) > > > - if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], topology)) > > > - return g_top_table[i].top_name; > > > - > > > - return DPU_RM_TOPOLOGY_NONE; > > > -} > > > - > > > void dpu_rm_init_hw_iter( > > > struct dpu_rm_hw_iter *iter, > > > uint32_t enc_id, > > > @@ -760,7 +745,6 @@ static int _dpu_rm_make_next_rsvp( > > > /* Create reservation info, tag reserved blocks with it as we go > > */ > > > rsvp->seq = ++rm->rsvp_next_seq; > > > rsvp->enc_id = enc->base.id; > > > - rsvp->topology = reqs->topology->top_name; > > > list_add_tail(&rsvp->list, &rm->rsvps); > > > > > > ret = _dpu_rm_reserve_lms(rm, rsvp, reqs); > > > @@ -807,7 +791,7 @@ static int _dpu_rm_populate_requirements( > > > > > > dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state); > > > > > > - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) { > > > + for (i = 0; i < ARRAY_SIZE(g_top_table); i++) { > > > if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], > > > req_topology)) { > > > reqs->topology = &g_top_table[i]; > > > > Since you can get rid of g_top_table, you can get rid of reqs->topology > > too. > > > DPU RM is using reqs->topology (ie. struct dpu_rm_topology_def) to represent > the hw_block requirements for a display. Removing the structure > might require changes in rest of RM alloc functions. > > If you can hold off on this, I am planning to address your comment in the > RM refactor series. I'd rather it be introduced when it's used. That way it can evalutated on its own merits when we have the full picture. There's a bunch of unused or duplicated code surrounding the RM, and I think it'd be best to clean it up while we all have full context in our heads. Sean > > Jeykumar S. > > > > @@ -831,9 +815,8 @@ static int _dpu_rm_populate_requirements( > > > > > > DRM_DEBUG_KMS("top_ctrl: 0x%llX num_h_tiles: %d\n", > > reqs->top_ctrl, > > > reqs->hw_res.display_num_of_h_tiles); > > > - DRM_DEBUG_KMS("num_lm: %d num_ctl: %d topology: %d split_display: > > %d\n", > > > + DRM_DEBUG_KMS("num_lm: %d num_ctl: %d split_display: %d\n", > > > reqs->topology->num_lm, reqs->topology->num_ctl, > > > - reqs->topology->top_name, > > > reqs->topology->needs_split_display); > > > > > > return 0; > > > @@ -969,8 +952,7 @@ static int _dpu_rm_commit_rsvp( > > > } > > > > > > if (!ret) > > > - DRM_DEBUG_KMS("rsrv enc %d topology %d\n", rsvp->enc_id, > > > - rsvp->topology); > > > + DRM_DEBUG_KMS("rsrv enc %d\n", rsvp->enc_id); > > > > This debug message doesn't provide any value any longer. > > > > > > > > return ret; > > > } > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > > > index ffd1841..6fd9e7b 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h > > > @@ -21,21 +21,6 @@ > > > #include "dpu_hw_top.h" > > > > > > /** > > > - * enum dpu_rm_topology_name - HW resource use case in use by > > > connector > > > - * @DPU_RM_TOPOLOGY_NONE: No topology in use > > > currently > > > - * @DPU_RM_TOPOLOGY_SINGLEPIPE: 1 LM, 1 PP, 1 INTF/WB > > > - * @DPU_RM_TOPOLOGY_DUALPIPE: 2 LM, 2 PP, 2 INTF/WB > > > - * @DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE: 2 LM, 2 PP, 3DMux, 1 > > > INTF/WB > > > - */ > > > -enum dpu_rm_topology_name { > > > - DPU_RM_TOPOLOGY_NONE = 0, > > > - DPU_RM_TOPOLOGY_SINGLEPIPE, > > > - DPU_RM_TOPOLOGY_DUALPIPE, > > > - DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, > > > - DPU_RM_TOPOLOGY_MAX, > > > -}; > > > - > > > -/** > > > * enum dpu_rm_topology_control - HW resource use case in use by > > connector > > > * @DPU_RM_TOPCTL_RESERVE_LOCK: If set, in AtomicTest phase, after a > > successful > > > * test, reserve the resources for this > > display. > > > @@ -187,13 +172,4 @@ void dpu_rm_init_hw_iter( > > > */ > > > int dpu_rm_check_property_topctl(uint64_t val); > > > > > > -/** > > > - * dpu_rm_get_topology_name - returns the name of the the given > > topology > > > - * definition > > > - * @topology: topology definition > > > - * @Return: name of the topology > > > - */ > > > -enum dpu_rm_topology_name > > > -dpu_rm_get_topology_name(struct msm_display_topology topology); > > > - > > > #endif /* __DPU_RM_H__ */ > > > -- > > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > > Forum, > > > a Linux Foundation Collaborative Project > > > > > -- > Jeykumar S
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index dbf669e..56ef349 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1013,7 +1013,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, struct drm_connector *conn = NULL, *conn_iter; struct dpu_rm_hw_iter pp_iter, ctl_iter; struct msm_display_topology topology; - enum dpu_rm_topology_name topology_name; struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL }; int i = 0, ret; @@ -1069,7 +1068,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, hw_ctl[i] = (struct dpu_hw_ctl *)ctl_iter.hw; } - topology_name = dpu_rm_get_topology_name(topology); for (i = 0; i < dpu_enc->num_phys_encs; i++) { struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i]; @@ -1089,7 +1087,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc, phys->hw_ctl = hw_ctl[i]; phys->connector = conn->state->connector; - phys->topology_name = topology_name; if (phys->ops.mode_set) phys->ops.mode_set(phys, mode, adj_mode); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h index 60f809f..c5600e6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h @@ -35,7 +35,6 @@ * @needs_cdm: Encoder requests a CDM based on pixel format conversion needs * @display_num_of_h_tiles: Number of horizontal tiles in case of split * interface - * @topology: Topology of the display */ struct dpu_encoder_hw_resources { enum dpu_intf_mode intfs[INTF_MAX]; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h index b3917e0..b5fc65c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h @@ -24,6 +24,7 @@ #include "dpu_hw_top.h" #include "dpu_hw_cdm.h" #include "dpu_encoder.h" +#include "dpu_crtc.h" #define DPU_ENCODER_NAME_MAX 16 @@ -213,7 +214,6 @@ struct dpu_encoder_irq { * @split_role: Role to play in a split-panel configuration * @intf_mode: Interface mode * @intf_idx: Interface index on dpu hardware - * @topology_name: topology selected for the display * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes * @enable_state: Enable state tracking * @vblank_refcount: Reference count of vblank request @@ -243,7 +243,6 @@ struct dpu_encoder_phys { enum dpu_enc_split_role split_role; enum dpu_intf_mode intf_mode; enum dpu_intf intf_idx; - enum dpu_rm_topology_name topology_name; spinlock_t *enc_spinlock; enum dpu_enc_enable_state enable_state; atomic_t vblank_refcount; @@ -361,11 +360,15 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init( static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode( struct dpu_encoder_phys *phys_enc) { + struct dpu_crtc_state *dpu_cstate; + if (!phys_enc || phys_enc->enable_state == DPU_ENC_DISABLING) return BLEND_3D_NONE; + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); + if (phys_enc->split_role == ENC_ROLE_SOLO && - phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE) + (dpu_cstate->num_mixers == 2)) return BLEND_3D_H_ROW_INT; return BLEND_3D_NONE; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 6de13f4..1643e2f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -355,13 +355,14 @@ static void dpu_encoder_phys_vid_underrun_irq(void *arg, int irq_idx) static bool _dpu_encoder_phys_is_dual_ctl(struct dpu_encoder_phys *phys_enc) { + struct dpu_crtc_state *dpu_cstate; + if (!phys_enc) return false; - if (phys_enc->topology_name == DPU_RM_TOPOLOGY_DUALPIPE) - return true; + dpu_cstate = to_dpu_crtc_state(phys_enc->parent->crtc->state); - return false; + return (dpu_cstate->num_ctls > 1); } static bool dpu_encoder_phys_vid_needs_single_flush( diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index 13c0a36..fbd489f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -33,7 +33,6 @@ (t).num_intf == (r).num_intf) struct dpu_rm_topology_def { - enum dpu_rm_topology_name top_name; int num_lm; int num_comp_enc; int num_intf; @@ -42,10 +41,10 @@ struct dpu_rm_topology_def { }; static const struct dpu_rm_topology_def g_top_table[] = { - { DPU_RM_TOPOLOGY_NONE, 0, 0, 0, 0, false }, - { DPU_RM_TOPOLOGY_SINGLEPIPE, 1, 0, 1, 1, false }, - { DPU_RM_TOPOLOGY_DUALPIPE, 2, 0, 2, 2, true }, - { DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, 2, 0, 1, 1, false }, + { 0, 0, 0, 0, false }, + { 1, 0, 1, 1, false }, + { 2, 0, 2, 2, true }, + { 2, 0, 1, 1, false }, }; /** @@ -72,13 +71,11 @@ struct dpu_rm_requirements { * @enc_id: Reservations are tracked by Encoder DRM object ID. * CRTCs may be connected to multiple Encoders. * An encoder or connector id identifies the display path. - * @topology DRM<->HW topology use case */ struct dpu_rm_rsvp { struct list_head list; uint32_t seq; uint32_t enc_id; - enum dpu_rm_topology_name topology; }; /** @@ -122,8 +119,8 @@ static void _dpu_rm_print_rsvps( DPU_DEBUG("%d\n", stage); list_for_each_entry(rsvp, &rm->rsvps, list) { - DRM_DEBUG_KMS("%d rsvp[s%ue%u] topology %d\n", stage, rsvp->seq, - rsvp->enc_id, rsvp->topology); + DRM_DEBUG_KMS("%d rsvp[s%ue%u]\n", stage, rsvp->seq, + rsvp->enc_id); } for (type = 0; type < DPU_HW_BLK_MAX; type++) { @@ -146,18 +143,6 @@ struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm) return rm->hw_mdp; } -enum dpu_rm_topology_name -dpu_rm_get_topology_name(struct msm_display_topology topology) -{ - int i; - - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) - if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], topology)) - return g_top_table[i].top_name; - - return DPU_RM_TOPOLOGY_NONE; -} - void dpu_rm_init_hw_iter( struct dpu_rm_hw_iter *iter, uint32_t enc_id, @@ -760,7 +745,6 @@ static int _dpu_rm_make_next_rsvp( /* Create reservation info, tag reserved blocks with it as we go */ rsvp->seq = ++rm->rsvp_next_seq; rsvp->enc_id = enc->base.id; - rsvp->topology = reqs->topology->top_name; list_add_tail(&rsvp->list, &rm->rsvps); ret = _dpu_rm_reserve_lms(rm, rsvp, reqs); @@ -807,7 +791,7 @@ static int _dpu_rm_populate_requirements( dpu_encoder_get_hw_resources(enc, &reqs->hw_res, conn_state); - for (i = 0; i < DPU_RM_TOPOLOGY_MAX; i++) { + for (i = 0; i < ARRAY_SIZE(g_top_table); i++) { if (RM_IS_TOPOLOGY_MATCH(g_top_table[i], req_topology)) { reqs->topology = &g_top_table[i]; @@ -831,9 +815,8 @@ static int _dpu_rm_populate_requirements( DRM_DEBUG_KMS("top_ctrl: 0x%llX num_h_tiles: %d\n", reqs->top_ctrl, reqs->hw_res.display_num_of_h_tiles); - DRM_DEBUG_KMS("num_lm: %d num_ctl: %d topology: %d split_display: %d\n", + DRM_DEBUG_KMS("num_lm: %d num_ctl: %d split_display: %d\n", reqs->topology->num_lm, reqs->topology->num_ctl, - reqs->topology->top_name, reqs->topology->needs_split_display); return 0; @@ -969,8 +952,7 @@ static int _dpu_rm_commit_rsvp( } if (!ret) - DRM_DEBUG_KMS("rsrv enc %d topology %d\n", rsvp->enc_id, - rsvp->topology); + DRM_DEBUG_KMS("rsrv enc %d\n", rsvp->enc_id); return ret; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h index ffd1841..6fd9e7b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h @@ -21,21 +21,6 @@ #include "dpu_hw_top.h" /** - * enum dpu_rm_topology_name - HW resource use case in use by connector - * @DPU_RM_TOPOLOGY_NONE: No topology in use currently - * @DPU_RM_TOPOLOGY_SINGLEPIPE: 1 LM, 1 PP, 1 INTF/WB - * @DPU_RM_TOPOLOGY_DUALPIPE: 2 LM, 2 PP, 2 INTF/WB - * @DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE: 2 LM, 2 PP, 3DMux, 1 INTF/WB - */ -enum dpu_rm_topology_name { - DPU_RM_TOPOLOGY_NONE = 0, - DPU_RM_TOPOLOGY_SINGLEPIPE, - DPU_RM_TOPOLOGY_DUALPIPE, - DPU_RM_TOPOLOGY_DUALPIPE_3DMERGE, - DPU_RM_TOPOLOGY_MAX, -}; - -/** * enum dpu_rm_topology_control - HW resource use case in use by connector * @DPU_RM_TOPCTL_RESERVE_LOCK: If set, in AtomicTest phase, after a successful * test, reserve the resources for this display. @@ -187,13 +172,4 @@ void dpu_rm_init_hw_iter( */ int dpu_rm_check_property_topctl(uint64_t val); -/** - * dpu_rm_get_topology_name - returns the name of the the given topology - * definition - * @topology: topology definition - * @Return: name of the topology - */ -enum dpu_rm_topology_name -dpu_rm_get_topology_name(struct msm_display_topology topology); - #endif /* __DPU_RM_H__ */
Strip down the support for topology enums. It can be replaced with simple hw count checks. Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 -- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 9 ++++-- .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 7 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 36 ++++++---------------- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 24 --------------- 6 files changed, 19 insertions(+), 61 deletions(-)