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 thisdisplay.
@@ -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