Discussion:
[PATCH v2 0/6] DPU resource manager cleanup
Jeykumar Sankaran
2018-12-08 02:38:32 UTC
Permalink
First set of clean up patches for DPU resource manager.
Removes/realigns some of the redudant RM interfaces.
Eventual plan is to migrate resource maintenence using
private state objects.

Thanks and Regards,
Jeykumar S

Jeykumar Sankaran (6):
drm/msm/dpu: avoid tracking reservations in RM
drm/msm/dpu: remove dev from RM
drm/msm/dpu: clean up dpu_rm_check_property_topctl declaration
drm/msm/dpu: remove encoder from crtc mixer struct
drm/msm/dpu: clean up redundant hw type
drm/msm/dpu: maintain hw_mdp in kms

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 -
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 325 +++++-------------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 28 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 +--
6 files changed, 66 insertions(+), 333 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jeykumar Sankaran
2018-12-08 02:38:33 UTC
Permalink
RM was equipped with reservation tracking structure RSVP
to cache HW reservation of displays for certain clients
where atomic_checks (atomic commit with TEST_ONLY) for all
the displays are called before their respective atomic_commits.
Since DPU doesn't support the sequence anymore, clean up
the support from RM. Replace rsvp with the corresponding
encoder id to tag the HW blocks reserved. It prepares DPU
to get rid of RM altogether and track reservations using
private states.

changes in v2:
- none

Signed-off-by: Jeykumar Sankaran <***@codeaurora.org>
Reviewed-by: Sean Paul <***@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 284 +++++----------------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 4 -
2 files changed, 43 insertions(+), 245 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index bdb1177..36a929b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -21,8 +21,8 @@
#include "dpu_encoder.h"
#include "dpu_trace.h"

-#define RESERVED_BY_OTHER(h, r) \
- ((h)->rsvp && ((h)->rsvp->enc_id != (r)->enc_id))
+#define RESERVED_BY_OTHER(h, r) \
+ ((h)->enc_id && (h)->enc_id != r)

/**
* struct dpu_rm_requirements - Reservation requirements parameter bundle
@@ -34,85 +34,23 @@ struct dpu_rm_requirements {
struct dpu_encoder_hw_resources hw_res;
};

-/**
- * struct dpu_rm_rsvp - Use Case Reservation tagging structure
- * Used to tag HW blocks as reserved by a CRTC->Encoder->Connector chain
- * By using as a tag, rather than lists of pointers to HW blocks used
- * we can avoid some list management since we don't know how many blocks
- * of each type a given use case may require.
- * @list: List head for list of all reservations
- * @seq: Global RSVP sequence number for debugging, especially for
- * differentiating differenct allocations for same encoder.
- * @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.
- */
-struct dpu_rm_rsvp {
- struct list_head list;
- uint32_t seq;
- uint32_t enc_id;
-};

/**
* struct dpu_rm_hw_blk - hardware block tracking list member
* @list: List head for list of all hardware blocks tracking items
- * @rsvp: Pointer to use case reservation if reserved by a client
- * @rsvp_nxt: Temporary pointer used during reservation to the incoming
- * request. Will be swapped into rsvp if proposal is accepted
* @type: Type of hardware block this structure tracks
* @id: Hardware ID number, within it's own space, ie. LM_X
- * @catalog: Pointer to the hardware catalog entry for this block
+ * @enc_id: Encoder id to which this blk is binded
* @hw: Pointer to the hardware register access object for this block
*/
struct dpu_rm_hw_blk {
struct list_head list;
- struct dpu_rm_rsvp *rsvp;
- struct dpu_rm_rsvp *rsvp_nxt;
enum dpu_hw_blk_type type;
uint32_t id;
+ uint32_t enc_id;
struct dpu_hw_blk *hw;
};

-/**
- * dpu_rm_dbg_rsvp_stage - enum of steps in making reservation for event logging
- */
-enum dpu_rm_dbg_rsvp_stage {
- DPU_RM_STAGE_BEGIN,
- DPU_RM_STAGE_AFTER_CLEAR,
- DPU_RM_STAGE_AFTER_RSVPNEXT,
- DPU_RM_STAGE_FINAL
-};
-
-static void _dpu_rm_print_rsvps(
- struct dpu_rm *rm,
- enum dpu_rm_dbg_rsvp_stage stage)
-{
- struct dpu_rm_rsvp *rsvp;
- struct dpu_rm_hw_blk *blk;
- enum dpu_hw_blk_type type;
-
- DPU_DEBUG("%d\n", stage);
-
- list_for_each_entry(rsvp, &rm->rsvps, list) {
- DRM_DEBUG_KMS("%d rsvp[s%ue%u]\n", stage, rsvp->seq,
- rsvp->enc_id);
- }
-
- for (type = 0; type < DPU_HW_BLK_MAX; type++) {
- list_for_each_entry(blk, &rm->hw_blks[type], list) {
- if (!blk->rsvp && !blk->rsvp_nxt)
- continue;
-
- DRM_DEBUG_KMS("%d rsvp[s%ue%u->s%ue%u] %d %d\n", stage,
- (blk->rsvp) ? blk->rsvp->seq : 0,
- (blk->rsvp) ? blk->rsvp->enc_id : 0,
- (blk->rsvp_nxt) ? blk->rsvp_nxt->seq : 0,
- (blk->rsvp_nxt) ? blk->rsvp_nxt->enc_id : 0,
- blk->type, blk->id);
- }
- }
-}
-
struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm)
{
return rm->hw_mdp;
@@ -148,15 +86,13 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm, struct dpu_rm_hw_iter *i)
i->blk = list_prepare_entry(i->blk, blk_list, list);

list_for_each_entry_continue(i->blk, blk_list, list) {
- struct dpu_rm_rsvp *rsvp = i->blk->rsvp;
-
if (i->blk->type != i->type) {
DPU_ERROR("found incorrect block type %d on %d list\n",
i->blk->type, i->type);
return false;
}

- if ((i->enc_id == 0) || (rsvp && rsvp->enc_id == i->enc_id)) {
+ if (i->enc_id == i->blk->enc_id) {
i->hw = i->blk->hw;
DPU_DEBUG("found type %d id %d for enc %d\n",
i->type, i->blk->id, i->enc_id);
@@ -208,22 +144,9 @@ static void _dpu_rm_hw_destroy(enum dpu_hw_blk_type type, void *hw)

int dpu_rm_destroy(struct dpu_rm *rm)
{
-
- struct dpu_rm_rsvp *rsvp_cur, *rsvp_nxt;
struct dpu_rm_hw_blk *hw_cur, *hw_nxt;
enum dpu_hw_blk_type type;

- if (!rm) {
- DPU_ERROR("invalid rm\n");
- return -EINVAL;
- }
-
- list_for_each_entry_safe(rsvp_cur, rsvp_nxt, &rm->rsvps, list) {
- list_del(&rsvp_cur->list);
- kfree(rsvp_cur);
- }
-
-
for (type = 0; type < DPU_HW_BLK_MAX; type++) {
list_for_each_entry_safe(hw_cur, hw_nxt, &rm->hw_blks[type],
list) {
@@ -293,6 +216,7 @@ static int _dpu_rm_hw_blk_create(
blk->type = type;
blk->id = id;
blk->hw = hw;
+ blk->enc_id = 0;
list_add_tail(&blk->list, &rm->hw_blks[type]);

return 0;
@@ -316,7 +240,6 @@ int dpu_rm_init(struct dpu_rm *rm,

mutex_init(&rm->rm_lock);

- INIT_LIST_HEAD(&rm->rsvps);
for (type = 0; type < DPU_HW_BLK_MAX; type++)
INIT_LIST_HEAD(&rm->hw_blks[type]);

@@ -410,7 +333,7 @@ static bool _dpu_rm_needs_split_display(const struct msm_display_topology *top)
* proposed use case requirements, incl. hardwired dependent blocks like
* pingpong
* @rm: dpu resource manager handle
- * @rsvp: reservation currently being created
+ * @enc_id: encoder id requesting for allocation
* @reqs: proposed use case requirements
* @lm: proposed layer mixer, function checks if lm, and all other hardwired
* blocks connected to the lm (pp) is available and appropriate
@@ -422,7 +345,7 @@ static bool _dpu_rm_needs_split_display(const struct msm_display_topology *top)
*/
static bool _dpu_rm_check_lm_and_get_connected_blks(
struct dpu_rm *rm,
- struct dpu_rm_rsvp *rsvp,
+ uint32_t enc_id,
struct dpu_rm_requirements *reqs,
struct dpu_rm_hw_blk *lm,
struct dpu_rm_hw_blk **pp,
@@ -449,7 +372,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
}

/* Already reserved? */
- if (RESERVED_BY_OTHER(lm, rsvp)) {
+ if (RESERVED_BY_OTHER(lm, enc_id)) {
DPU_DEBUG("lm %d already reserved\n", lm_cfg->id);
return false;
}
@@ -467,7 +390,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
return false;
}

- if (RESERVED_BY_OTHER(*pp, rsvp)) {
+ if (RESERVED_BY_OTHER(*pp, enc_id)) {
DPU_DEBUG("lm %d pp %d already reserved\n", lm->id,
(*pp)->id);
return false;
@@ -476,10 +399,8 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
return true;
}

-static int _dpu_rm_reserve_lms(
- struct dpu_rm *rm,
- struct dpu_rm_rsvp *rsvp,
- struct dpu_rm_requirements *reqs)
+static int _dpu_rm_reserve_lms(struct dpu_rm *rm, uint32_t enc_id,
+ struct dpu_rm_requirements *reqs)

{
struct dpu_rm_hw_blk *lm[MAX_BLOCKS];
@@ -504,7 +425,7 @@ static int _dpu_rm_reserve_lms(
lm[lm_count] = iter_i.blk;

if (!_dpu_rm_check_lm_and_get_connected_blks(
- rm, rsvp, reqs, lm[lm_count],
+ rm, enc_id, reqs, lm[lm_count],
&pp[lm_count], NULL))
continue;

@@ -519,7 +440,7 @@ static int _dpu_rm_reserve_lms(
continue;

if (!_dpu_rm_check_lm_and_get_connected_blks(
- rm, rsvp, reqs, iter_j.blk,
+ rm, enc_id, reqs, iter_j.blk,
&pp[lm_count], iter_i.blk))
continue;

@@ -537,10 +458,10 @@ static int _dpu_rm_reserve_lms(
if (!lm[i])
break;

- lm[i]->rsvp_nxt = rsvp;
- pp[i]->rsvp_nxt = rsvp;
+ lm[i]->enc_id = enc_id;
+ pp[i]->enc_id = enc_id;

- trace_dpu_rm_reserve_lms(lm[i]->id, lm[i]->type, rsvp->enc_id,
+ trace_dpu_rm_reserve_lms(lm[i]->id, lm[i]->type, enc_id,
pp[i]->id);
}

@@ -549,7 +470,7 @@ static int _dpu_rm_reserve_lms(

static int _dpu_rm_reserve_ctls(
struct dpu_rm *rm,
- struct dpu_rm_rsvp *rsvp,
+ uint32_t enc_id,
const struct msm_display_topology *top)
{
struct dpu_rm_hw_blk *ctls[MAX_BLOCKS];
@@ -570,7 +491,7 @@ static int _dpu_rm_reserve_ctls(
unsigned long features = ctl->caps->features;
bool has_split_display;

- if (RESERVED_BY_OTHER(iter.blk, rsvp))
+ if (RESERVED_BY_OTHER(iter.blk, enc_id))
continue;

has_split_display = BIT(DPU_CTL_SPLIT_DISPLAY) & features;
@@ -591,9 +512,9 @@ static int _dpu_rm_reserve_ctls(
return -ENAVAIL;

for (i = 0; i < ARRAY_SIZE(ctls) && i < num_ctls; i++) {
- ctls[i]->rsvp_nxt = rsvp;
+ ctls[i]->enc_id = enc_id;
trace_dpu_rm_reserve_ctls(ctls[i]->id, ctls[i]->type,
- rsvp->enc_id);
+ enc_id);
}

return 0;
@@ -601,7 +522,7 @@ static int _dpu_rm_reserve_ctls(

static int _dpu_rm_reserve_intf(
struct dpu_rm *rm,
- struct dpu_rm_rsvp *rsvp,
+ uint32_t enc_id,
uint32_t id,
enum dpu_hw_blk_type type)
{
@@ -614,14 +535,14 @@ static int _dpu_rm_reserve_intf(
if (iter.blk->id != id)
continue;

- if (RESERVED_BY_OTHER(iter.blk, rsvp)) {
+ if (RESERVED_BY_OTHER(iter.blk, enc_id)) {
DPU_ERROR("type %d id %d already reserved\n", type, id);
return -ENAVAIL;
}

- iter.blk->rsvp_nxt = rsvp;
+ iter.blk->enc_id = enc_id;
trace_dpu_rm_reserve_intf(iter.blk->id, iter.blk->type,
- rsvp->enc_id);
+ enc_id);
break;
}

@@ -636,7 +557,7 @@ static int _dpu_rm_reserve_intf(

static int _dpu_rm_reserve_intf_related_hw(
struct dpu_rm *rm,
- struct dpu_rm_rsvp *rsvp,
+ uint32_t enc_id,
struct dpu_encoder_hw_resources *hw_res)
{
int i, ret = 0;
@@ -646,7 +567,7 @@ static int _dpu_rm_reserve_intf_related_hw(
if (hw_res->intfs[i] == INTF_MODE_NONE)
continue;
id = i + INTF_0;
- ret = _dpu_rm_reserve_intf(rm, rsvp, id,
+ ret = _dpu_rm_reserve_intf(rm, enc_id, id,
DPU_HW_BLK_INTF);
if (ret)
return ret;
@@ -655,33 +576,27 @@ static int _dpu_rm_reserve_intf_related_hw(
return ret;
}

-static int _dpu_rm_make_next_rsvp(
+static int _dpu_rm_make_reservation(
struct dpu_rm *rm,
struct drm_encoder *enc,
struct drm_crtc_state *crtc_state,
- struct dpu_rm_rsvp *rsvp,
struct dpu_rm_requirements *reqs)
{
int ret;

- /* Create reservation info, tag reserved blocks with it as we go */
- rsvp->seq = ++rm->rsvp_next_seq;
- rsvp->enc_id = enc->base.id;
- list_add_tail(&rsvp->list, &rm->rsvps);
-
- ret = _dpu_rm_reserve_lms(rm, rsvp, reqs);
+ ret = _dpu_rm_reserve_lms(rm, enc->base.id, reqs);
if (ret) {
DPU_ERROR("unable to find appropriate mixers\n");
return ret;
}

- ret = _dpu_rm_reserve_ctls(rm, rsvp, &reqs->topology);
+ ret = _dpu_rm_reserve_ctls(rm, enc->base.id, &reqs->topology);
if (ret) {
DPU_ERROR("unable to find appropriate CTL\n");
return ret;
}

- ret = _dpu_rm_reserve_intf_related_hw(rm, rsvp, &reqs->hw_res);
+ ret = _dpu_rm_reserve_intf_related_hw(rm, enc->base.id, &reqs->hw_res);
if (ret)
return ret;

@@ -706,108 +621,31 @@ static int _dpu_rm_populate_requirements(
return 0;
}

-static struct dpu_rm_rsvp *_dpu_rm_get_rsvp(
- struct dpu_rm *rm,
- struct drm_encoder *enc)
+static void _dpu_rm_release_reservation(struct dpu_rm *rm, uint32_t enc_id)
{
- struct dpu_rm_rsvp *i;
-
- if (!rm || !enc) {
- DPU_ERROR("invalid params\n");
- return NULL;
- }
-
- if (list_empty(&rm->rsvps))
- return NULL;
-
- list_for_each_entry(i, &rm->rsvps, list)
- if (i->enc_id == enc->base.id)
- return i;
-
- return NULL;
-}
-
-/**
- * _dpu_rm_release_rsvp - release resources and release a reservation
- * @rm: KMS handle
- * @rsvp: RSVP pointer to release and release resources for
- */
-static void _dpu_rm_release_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
-{
- struct dpu_rm_rsvp *rsvp_c, *rsvp_n;
struct dpu_rm_hw_blk *blk;
enum dpu_hw_blk_type type;

- if (!rsvp)
- return;
-
- DPU_DEBUG("rel rsvp %d enc %d\n", rsvp->seq, rsvp->enc_id);
-
- list_for_each_entry_safe(rsvp_c, rsvp_n, &rm->rsvps, list) {
- if (rsvp == rsvp_c) {
- list_del(&rsvp_c->list);
- break;
- }
- }
-
for (type = 0; type < DPU_HW_BLK_MAX; type++) {
list_for_each_entry(blk, &rm->hw_blks[type], list) {
- if (blk->rsvp == rsvp) {
- blk->rsvp = NULL;
- DPU_DEBUG("rel rsvp %d enc %d %d %d\n",
- rsvp->seq, rsvp->enc_id,
- blk->type, blk->id);
- }
- if (blk->rsvp_nxt == rsvp) {
- blk->rsvp_nxt = NULL;
- DPU_DEBUG("rel rsvp_nxt %d enc %d %d %d\n",
- rsvp->seq, rsvp->enc_id,
- blk->type, blk->id);
+ if (blk->enc_id == enc_id) {
+ blk->enc_id = 0;
+ DPU_DEBUG("rel enc %d %d %d\n", enc_id,
+ blk->type, blk->id);
}
}
}
-
- kfree(rsvp);
}

void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
{
- struct dpu_rm_rsvp *rsvp;
-
- if (!rm || !enc) {
- DPU_ERROR("invalid params\n");
- return;
- }
-
mutex_lock(&rm->rm_lock);

- rsvp = _dpu_rm_get_rsvp(rm, enc);
- if (!rsvp) {
- DPU_ERROR("failed to find rsvp for enc %d\n", enc->base.id);
- goto end;
- }
+ _dpu_rm_release_reservation(rm, enc->base.id);

- _dpu_rm_release_rsvp(rm, rsvp);
-end:
mutex_unlock(&rm->rm_lock);
}

-static void _dpu_rm_commit_rsvp(struct dpu_rm *rm, struct dpu_rm_rsvp *rsvp)
-{
- struct dpu_rm_hw_blk *blk;
- enum dpu_hw_blk_type type;
-
- /* Swap next rsvp to be the active */
- for (type = 0; type < DPU_HW_BLK_MAX; type++) {
- list_for_each_entry(blk, &rm->hw_blks[type], list) {
- if (blk->rsvp_nxt) {
- blk->rsvp = blk->rsvp_nxt;
- blk->rsvp_nxt = NULL;
- }
- }
- }
-}
-
int dpu_rm_reserve(
struct dpu_rm *rm,
struct drm_encoder *enc,
@@ -815,7 +653,6 @@ int dpu_rm_reserve(
struct msm_display_topology topology,
bool test_only)
{
- struct dpu_rm_rsvp *rsvp_cur, *rsvp_nxt;
struct dpu_rm_requirements reqs;
int ret;

@@ -828,8 +665,6 @@ int dpu_rm_reserve(

mutex_lock(&rm->rm_lock);

- _dpu_rm_print_rsvps(rm, DPU_RM_STAGE_BEGIN);
-
ret = _dpu_rm_populate_requirements(rm, enc, crtc_state, &reqs,
topology);
if (ret) {
@@ -837,50 +672,17 @@ int dpu_rm_reserve(
goto end;
}

- /*
- * We only support one active reservation per-hw-block. But to implement
- * transactional semantics for test-only, and for allowing failure while
- * modifying your existing reservation, over the course of this
- * function we can have two reservations:
- * Current: Existing reservation
- * Next: Proposed reservation. The proposed reservation may fail, or may
- * be discarded if in test-only mode.
- * If reservation is successful, and we're not in test-only, then we
- * replace the current with the next.
- */
- rsvp_nxt = kzalloc(sizeof(*rsvp_nxt), GFP_KERNEL);
- if (!rsvp_nxt) {
- ret = -ENOMEM;
- goto end;
- }
-
- rsvp_cur = _dpu_rm_get_rsvp(rm, enc);
-
- /* Check the proposed reservation, store it in hw's "next" field */
- ret = _dpu_rm_make_next_rsvp(rm, enc, crtc_state, rsvp_nxt, &reqs);
-
- _dpu_rm_print_rsvps(rm, DPU_RM_STAGE_AFTER_RSVPNEXT);
-
+ ret = _dpu_rm_make_reservation(rm, enc, crtc_state, &reqs);
if (ret) {
DPU_ERROR("failed to reserve hw resources: %d\n", ret);
- _dpu_rm_release_rsvp(rm, rsvp_nxt);
+ _dpu_rm_release_reservation(rm, enc->base.id);
} else if (test_only) {
- /*
- * Normally, if test_only, test the reservation and then undo
- * However, if the user requests LOCK, then keep the reservation
- * made during the atomic_check phase.
- */
- DPU_DEBUG("test_only: discard test rsvp[s%de%d]\n",
- rsvp_nxt->seq, rsvp_nxt->enc_id);
- _dpu_rm_release_rsvp(rm, rsvp_nxt);
- } else {
- _dpu_rm_release_rsvp(rm, rsvp_cur);
-
- _dpu_rm_commit_rsvp(rm, rsvp_nxt);
+ /* test_only: test the reservation and then undo */
+ DPU_DEBUG("test_only: discard test [enc: %d]\n",
+ enc->base.id);
+ _dpu_rm_release_reservation(rm, enc->base.id);
}

- _dpu_rm_print_rsvps(rm, DPU_RM_STAGE_FINAL);
-
end:
mutex_unlock(&rm->rm_lock);

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index b8273bd..0dd3c21 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -23,21 +23,17 @@
/**
* struct dpu_rm - DPU dynamic hardware resource manager
* @dev: device handle for event logging purposes
- * @rsvps: list of hardware reservations by each crtc->encoder->connector
* @hw_blks: array of lists of hardware resources present in the system, one
* list per type of hardware block
* @hw_mdp: hardware object for mdp_top
* @lm_max_width: cached layer mixer maximum width
- * @rsvp_next_seq: sequence number for next reservation for debugging purposes
* @rm_lock: resource manager mutex
*/
struct dpu_rm {
struct drm_device *dev;
- struct list_head rsvps;
struct list_head hw_blks[DPU_HW_BLK_MAX];
struct dpu_hw_mdp *hw_mdp;
uint32_t lm_max_width;
- uint32_t rsvp_next_seq;
struct mutex rm_lock;
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jeykumar Sankaran
2018-12-08 02:38:34 UTC
Permalink
Not used. Remove from RM.

changes in v2:
- none

Signed-off-by: Jeykumar Sankaran <***@codeaurora.org>
Reviewed-by: Sean Paul <***@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 3 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 ++-----
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 6 +-----
3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 4d67b3c..1ecd04a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -877,8 +877,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
goto power_error;
}

- rc = dpu_rm_init(&dpu_kms->rm, dpu_kms->catalog, dpu_kms->mmio,
- dpu_kms->dev);
+ rc = dpu_rm_init(&dpu_kms->rm, dpu_kms->catalog, dpu_kms->mmio);
if (rc) {
DPU_ERROR("rm init failed: %d\n", rc);
goto power_error;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 36a929b..5ce89b9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -224,13 +224,12 @@ static int _dpu_rm_hw_blk_create(

int dpu_rm_init(struct dpu_rm *rm,
struct dpu_mdss_cfg *cat,
- void __iomem *mmio,
- struct drm_device *dev)
+ void __iomem *mmio)
{
int rc, i;
enum dpu_hw_blk_type type;

- if (!rm || !cat || !mmio || !dev) {
+ if (!rm || !cat || !mmio) {
DPU_ERROR("invalid kms\n");
return -EINVAL;
}
@@ -243,8 +242,6 @@ int dpu_rm_init(struct dpu_rm *rm,
for (type = 0; type < DPU_HW_BLK_MAX; type++)
INIT_LIST_HEAD(&rm->hw_blks[type]);

- rm->dev = dev;
-
/* Some of the sub-blocks require an mdptop to be created */
rm->hw_mdp = dpu_hw_mdptop_init(MDP_TOP, mmio, cat);
if (IS_ERR_OR_NULL(rm->hw_mdp)) {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 0dd3c21..f41fd19 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -22,7 +22,6 @@

/**
* struct dpu_rm - DPU dynamic hardware resource manager
- * @dev: device handle for event logging purposes
* @hw_blks: array of lists of hardware resources present in the system, one
* list per type of hardware block
* @hw_mdp: hardware object for mdp_top
@@ -30,7 +29,6 @@
* @rm_lock: resource manager mutex
*/
struct dpu_rm {
- struct drm_device *dev;
struct list_head hw_blks[DPU_HW_BLK_MAX];
struct dpu_hw_mdp *hw_mdp;
uint32_t lm_max_width;
@@ -63,13 +61,11 @@ struct dpu_rm_hw_iter {
* @rm: DPU Resource Manager handle
* @cat: Pointer to hardware catalog
* @mmio: mapped register io address of MDP
- * @dev: device handle for event logging purposes
* @Return: 0 on Success otherwise -ERROR
*/
int dpu_rm_init(struct dpu_rm *rm,
struct dpu_mdss_cfg *cat,
- void __iomem *mmio,
- struct drm_device *dev);
+ void __iomem *mmio);

/**
* dpu_rm_destroy - Free all memory allocated by dpu_rm_init
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jeykumar Sankaran
2018-12-08 02:38:35 UTC
Permalink
Definition was removed already. Clean up header declaration.

changes in v2:
- none

Signed-off-by: Jeykumar Sankaran <***@codeaurora.org>
Reviewed-by: Sean Paul <***@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 8 --------
1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index f41fd19..eb6a6ac 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -136,12 +136,4 @@ void dpu_rm_init_hw_iter(
* @Return: true on match found, false on no match found
*/
bool dpu_rm_get_hw(struct dpu_rm *rm, struct dpu_rm_hw_iter *iter);
-
-/**
- * dpu_rm_check_property_topctl - validate property bitmask before it is set
- * @val: user's proposed topology control bitmask
- * @Return: 0 on success or error
- */
-int dpu_rm_check_property_topctl(uint64_t val);
-
#endif /* __DPU_RM_H__ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jeykumar Sankaran
2018-12-08 02:38:36 UTC
Permalink
Not actively used. Clean up the crtc mixer struct.

changes in v2:
- none

Signed-off-by: Jeykumar Sankaran <***@codeaurora.org>
Reviewed-by: Sean Paul <***@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 --
2 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9be7c35..d6e6925 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -465,8 +465,6 @@ static void _dpu_crtc_setup_mixer_for_encoder(
return;
}

- mixer->encoder = enc;
-
cstate->num_mixers++;
DPU_DEBUG("setup mixer %d: lm %d\n",
i, mixer->hw_lm->idx - LM_0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index dbfb38a..e59d62b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -84,14 +84,12 @@ struct dpu_crtc_smmu_state_data {
* struct dpu_crtc_mixer: stores the map for each virtual pipeline in the CRTC
* @hw_lm: LM HW Driver context
* @lm_ctl: CTL Path HW driver context
- * @encoder: Encoder attached to this lm & ctl
* @mixer_op_mode: mixer blending operation mode
* @flush_mask: mixer flush mask for ctl, mixer and pipe
*/
struct dpu_crtc_mixer {
struct dpu_hw_mixer *hw_lm;
struct dpu_hw_ctl *lm_ctl;
- struct drm_encoder *encoder;
u32 mixer_op_mode;
u32 flush_mask;
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jeykumar Sankaran
2018-12-08 02:38:37 UTC
Permalink
struct dpu_hw_blk has hw block type info. Remove duplicate
type tracking in struct dpu_rm_hw_blk.

changes in v2:
- remove redundant type in trace api's (Sean Paul)

Signed-off-by: Jeykumar Sankaran <***@codeaurora.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 22 +++++-----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 +++++++++++-----------------
2 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 5ce89b9..e7fc9a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -38,14 +38,12 @@ struct dpu_rm_requirements {
/**
* struct dpu_rm_hw_blk - hardware block tracking list member
* @list: List head for list of all hardware blocks tracking items
- * @type: Type of hardware block this structure tracks
* @id: Hardware ID number, within it's own space, ie. LM_X
* @enc_id: Encoder id to which this blk is binded
* @hw: Pointer to the hardware register access object for this block
*/
struct dpu_rm_hw_blk {
struct list_head list;
- enum dpu_hw_blk_type type;
uint32_t id;
uint32_t enc_id;
struct dpu_hw_blk *hw;
@@ -86,12 +84,6 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm, struct dpu_rm_hw_iter *i)
i->blk = list_prepare_entry(i->blk, blk_list, list);

list_for_each_entry_continue(i->blk, blk_list, list) {
- if (i->blk->type != i->type) {
- DPU_ERROR("found incorrect block type %d on %d list\n",
- i->blk->type, i->type);
- return false;
- }
-
if (i->enc_id == i->blk->enc_id) {
i->hw = i->blk->hw;
DPU_DEBUG("found type %d id %d for enc %d\n",
@@ -151,7 +143,7 @@ int dpu_rm_destroy(struct dpu_rm *rm)
list_for_each_entry_safe(hw_cur, hw_nxt, &rm->hw_blks[type],
list) {
list_del(&hw_cur->list);
- _dpu_rm_hw_destroy(hw_cur->type, hw_cur->hw);
+ _dpu_rm_hw_destroy(type, hw_cur->hw);
kfree(hw_cur);
}
}
@@ -213,7 +205,6 @@ static int _dpu_rm_hw_blk_create(
return -ENOMEM;
}

- blk->type = type;
blk->id = id;
blk->hw = hw;
blk->enc_id = 0;
@@ -458,8 +449,7 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm, uint32_t enc_id,
lm[i]->enc_id = enc_id;
pp[i]->enc_id = enc_id;

- trace_dpu_rm_reserve_lms(lm[i]->id, lm[i]->type, enc_id,
- pp[i]->id);
+ trace_dpu_rm_reserve_lms(lm[i]->id, enc_id, pp[i]->id);
}

return rc;
@@ -510,8 +500,7 @@ static int _dpu_rm_reserve_ctls(

for (i = 0; i < ARRAY_SIZE(ctls) && i < num_ctls; i++) {
ctls[i]->enc_id = enc_id;
- trace_dpu_rm_reserve_ctls(ctls[i]->id, ctls[i]->type,
- enc_id);
+ trace_dpu_rm_reserve_ctls(ctls[i]->id, enc_id);
}

return 0;
@@ -538,8 +527,7 @@ static int _dpu_rm_reserve_intf(
}

iter.blk->enc_id = enc_id;
- trace_dpu_rm_reserve_intf(iter.blk->id, iter.blk->type,
- enc_id);
+ trace_dpu_rm_reserve_intf(iter.blk->id, enc_id);
break;
}

@@ -628,7 +616,7 @@ static void _dpu_rm_release_reservation(struct dpu_rm *rm, uint32_t enc_id)
if (blk->enc_id == enc_id) {
blk->enc_id = 0;
DPU_DEBUG("rel enc %d %d %d\n", enc_id,
- blk->type, blk->id);
+ type, blk->id);
}
}
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index c78b521..8bb4609 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -831,48 +831,42 @@
);

DECLARE_EVENT_CLASS(dpu_rm_iter_template,
- TP_PROTO(uint32_t id, enum dpu_hw_blk_type type, uint32_t enc_id),
- TP_ARGS(id, type, enc_id),
+ TP_PROTO(uint32_t id, uint32_t enc_id),
+ TP_ARGS(id, enc_id),
TP_STRUCT__entry(
__field( uint32_t, id )
- __field( enum dpu_hw_blk_type, type )
__field( uint32_t, enc_id )
),
TP_fast_assign(
__entry->id = id;
- __entry->type = type;
__entry->enc_id = enc_id;
),
- TP_printk("id:%d type:%d enc_id:%u", __entry->id, __entry->type,
- __entry->enc_id)
+ TP_printk("id:%d enc_id:%u", __entry->id, __entry->enc_id)
);
DEFINE_EVENT(dpu_rm_iter_template, dpu_rm_reserve_intf,
- TP_PROTO(uint32_t id, enum dpu_hw_blk_type type, uint32_t enc_id),
- TP_ARGS(id, type, enc_id)
+ TP_PROTO(uint32_t id, uint32_t enc_id),
+ TP_ARGS(id, enc_id)
);
DEFINE_EVENT(dpu_rm_iter_template, dpu_rm_reserve_ctls,
- TP_PROTO(uint32_t id, enum dpu_hw_blk_type type, uint32_t enc_id),
- TP_ARGS(id, type, enc_id)
+ TP_PROTO(uint32_t id, uint32_t enc_id),
+ TP_ARGS(id, enc_id)
);

TRACE_EVENT(dpu_rm_reserve_lms,
- TP_PROTO(uint32_t id, enum dpu_hw_blk_type type, uint32_t enc_id,
- uint32_t pp_id),
- TP_ARGS(id, type, enc_id, pp_id),
+ TP_PROTO(uint32_t id, uint32_t enc_id, uint32_t pp_id),
+ TP_ARGS(id, enc_id, pp_id),
TP_STRUCT__entry(
__field( uint32_t, id )
- __field( enum dpu_hw_blk_type, type )
__field( uint32_t, enc_id )
__field( uint32_t, pp_id )
),
TP_fast_assign(
__entry->id = id;
- __entry->type = type;
__entry->enc_id = enc_id;
__entry->pp_id = pp_id;
),
- TP_printk("id:%d type:%d enc_id:%u pp_id:%u", __entry->id,
- __entry->type, __entry->enc_id, __entry->pp_id)
+ TP_printk("id:%d enc_id:%u pp_id:%u", __entry->id,
+ __entry->enc_id, __entry->pp_id)
);

TRACE_EVENT(dpu_vbif_wait_xin_halt_fail,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jeykumar Sankaran
2018-12-08 02:38:38 UTC
Permalink
hw_mdp block is common for displays. No need
to reserve per display.

changes in v2:
- use IS_ERR for error checking (Jordan Crouse)

Signed-off-by: Jeykumar Sankaran <***@codeaurora.org>
Reviewed-by: Sean Paul <***@chromium.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 11 +++++++----
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 20 --------------------
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 10 ----------
3 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1ecd04a..f489526 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -627,6 +627,10 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
devm_iounmap(&dpu_kms->pdev->dev, dpu_kms->vbif[VBIF_RT]);
dpu_kms->vbif[VBIF_RT] = NULL;

+ if (dpu_kms->hw_mdp)
+ dpu_hw_mdp_destroy(dpu_kms->hw_mdp);
+ dpu_kms->hw_mdp = NULL;
+
if (dpu_kms->mmio)
devm_iounmap(&dpu_kms->pdev->dev, dpu_kms->mmio);
dpu_kms->mmio = NULL;
@@ -885,11 +889,10 @@ static int dpu_kms_hw_init(struct msm_kms *kms)

dpu_kms->rm_init = true;

- dpu_kms->hw_mdp = dpu_rm_get_mdp(&dpu_kms->rm);
- if (IS_ERR_OR_NULL(dpu_kms->hw_mdp)) {
+ dpu_kms->hw_mdp = dpu_hw_mdptop_init(MDP_TOP, dpu_kms->mmio,
+ dpu_kms->catalog);
+ if (IS_ERR(dpu_kms->hw_mdp)) {
rc = PTR_ERR(dpu_kms->hw_mdp);
- if (!dpu_kms->hw_mdp)
- rc = -EINVAL;
DPU_ERROR("failed to get hw_mdp: %d\n", rc);
dpu_kms->hw_mdp = NULL;
goto power_error;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index e7fc9a4..037d9f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -49,11 +49,6 @@ struct dpu_rm_hw_blk {
struct dpu_hw_blk *hw;
};

-struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm)
-{
- return rm->hw_mdp;
-}
-
void dpu_rm_init_hw_iter(
struct dpu_rm_hw_iter *iter,
uint32_t enc_id,
@@ -148,9 +143,6 @@ int dpu_rm_destroy(struct dpu_rm *rm)
}
}

- dpu_hw_mdp_destroy(rm->hw_mdp);
- rm->hw_mdp = NULL;
-
mutex_destroy(&rm->rm_lock);

return 0;
@@ -165,11 +157,8 @@ static int _dpu_rm_hw_blk_create(
void *hw_catalog_info)
{
struct dpu_rm_hw_blk *blk;
- struct dpu_hw_mdp *hw_mdp;
void *hw;

- hw_mdp = rm->hw_mdp;
-
switch (type) {
case DPU_HW_BLK_LM:
hw = dpu_hw_lm_init(id, mmio, cat);
@@ -233,15 +222,6 @@ int dpu_rm_init(struct dpu_rm *rm,
for (type = 0; type < DPU_HW_BLK_MAX; type++)
INIT_LIST_HEAD(&rm->hw_blks[type]);

- /* Some of the sub-blocks require an mdptop to be created */
- rm->hw_mdp = dpu_hw_mdptop_init(MDP_TOP, mmio, cat);
- if (IS_ERR_OR_NULL(rm->hw_mdp)) {
- rc = PTR_ERR(rm->hw_mdp);
- rm->hw_mdp = NULL;
- DPU_ERROR("failed: mdp hw not available\n");
- goto fail;
- }
-
/* Interrogate HW catalog and create tracking items for hw blocks */
for (i = 0; i < cat->mixer_count; i++) {
struct dpu_lm_cfg *lm = &cat->mixer[i];
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index eb6a6ac..381611f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -24,13 +24,11 @@
* struct dpu_rm - DPU dynamic hardware resource manager
* @hw_blks: array of lists of hardware resources present in the system, one
* list per type of hardware block
- * @hw_mdp: hardware object for mdp_top
* @lm_max_width: cached layer mixer maximum width
* @rm_lock: resource manager mutex
*/
struct dpu_rm {
struct list_head hw_blks[DPU_HW_BLK_MAX];
- struct dpu_hw_mdp *hw_mdp;
uint32_t lm_max_width;
struct mutex rm_lock;
};
@@ -104,14 +102,6 @@ int dpu_rm_reserve(struct dpu_rm *rm,
void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc);

/**
- * dpu_rm_get_mdp - Retrieve HW block for MDP TOP.
- * This is never reserved, and is usable by any display.
- * @rm: DPU Resource Manager handle
- * @Return: Pointer to hw block or NULL
- */
-struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm);
-
-/**
* dpu_rm_init_hw_iter - setup given iterator for new iteration over hw list
* using dpu_rm_get_hw
* @iter: iter object to initialize
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Loading...