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 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