On Mon, Oct 08, 2018 at 09:27:39PM -0700, Jeykumar Sankaran wrote:
Instead of letting encoder make a centralized reservation for
all of its display DRM components, this path splits the
responsibility between CRTC and Encoder, each requesting
RM for the HW mapping of its own domain.
Signed-off-by: Jeykumar Sankaran <jsa...@codeaurora.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 31 +++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++----
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 69
++++++++++++++++++++++++-----
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 36 +++++++++++----
4 files changed, 119 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0625f56..0536b8a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -47,6 +47,8 @@
#define LEFT_MIXER 0
#define RIGHT_MIXER 1
+#define MAX_VDISPLAY_SPLIT 1080
+
static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state
*cstate,
struct drm_display_mode *mode)
{
@@ -448,6 +450,7 @@ static void _dpu_crtc_setup_lm_bounds(struct
drm_crtc *crtc,
for (i = 0; i < cstate->num_mixers; i++) {
struct drm_rect *r = &cstate->lm_bounds[i];
+
r->x1 = crtc_split_width * i;
r->y1 = 0;
r->x2 = r->x1 + crtc_split_width;
@@ -885,6 +888,7 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
struct drm_display_mode *mode;
struct drm_encoder *encoder;
struct msm_drm_private *priv;
+ struct dpu_kms *dpu_kms;
unsigned long flags;
if (!crtc || !crtc->dev || !crtc->dev->dev_private ||
!crtc->state) {
@@ -895,6 +899,7 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
cstate = to_dpu_crtc_state(crtc->state);
mode = &cstate->base.adjusted_mode;
priv = crtc->dev->dev_private;
+ dpu_kms = to_dpu_kms(priv->kms);
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
@@ -953,6 +958,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
crtc->state->event = NULL;
spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
}
+
+ dpu_rm_crtc_release(&dpu_kms->rm, crtc->state);
}
static void dpu_crtc_enable(struct drm_crtc *crtc,
@@ -1004,6 +1011,21 @@ struct plane_state {
u32 pipe_id;
};
+static void _dpu_crtc_get_topology(
+ struct drm_crtc_state *crtc_state,
+ struct drm_display_mode *mode)
+{
+ struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
+
+ dpu_cstate->num_mixers = (mode->vdisplay > MAX_VDISPLAY_SPLIT) ? 2
: 1;
+
+ /**
+ * encoder->atomic_check is invoked before crtc->atomic_check.
+ * so dpu_cstate->num_intfs should have a non-zero value.
+ */
+ dpu_cstate->num_ctls = dpu_cstate->num_intfs;
Why do we need num_ctls? Can't we just use dpu_cstate->num_intfs
directly?
Also,
you don't really need these in their own function, especially if
num_ctls
goes
away.
+}
+
static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
@@ -1014,6 +1036,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc
*crtc,
const struct drm_plane_state *pstate;
struct drm_plane *plane;
struct drm_display_mode *mode;
+ struct msm_drm_private *priv;
+ struct dpu_kms *dpu_kms;
int cnt = 0, rc = 0, mixer_width, i, z_pos;
@@ -1039,6 +1063,9 @@ static int dpu_crtc_atomic_check(struct drm_crtc
*crtc,
goto end;
}
+ priv = crtc->dev->dev_private;
+ dpu_kms = to_dpu_kms(priv->kms);
+
mode = &state->adjusted_mode;
DPU_DEBUG("%s: check", dpu_crtc->name);
@@ -1229,6 +1256,10 @@ static int dpu_crtc_atomic_check(struct
drm_crtc
*crtc,
}
}
+ _dpu_crtc_get_topology(state, mode);
+ if (drm_atomic_crtc_needs_modeset(state))
+ rc = dpu_rm_crtc_reserve(&dpu_kms->rm, state);
+
end:
kfree(pstates);
return rc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5d501c8..ce66309 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -67,8 +67,6 @@
#define IDLE_SHORT_TIMEOUT 1
-#define MAX_VDISPLAY_SPLIT 1080
-
/**
* enum dpu_enc_rc_events - events for resource control state machine
* @DPU_ENC_RC_EVENT_KICKOFF:
@@ -557,14 +555,10 @@ static void _dpu_encoder_adjust_mode(struct
drm_connector *connector,
static void _dpu_encoder_get_topology(
struct dpu_encoder_virt *dpu_enc,
- struct drm_crtc_state *crtc_state,
- struct drm_display_mode *mode)
+ struct drm_crtc_state *crtc_state)
{
struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
- /* User split topology for width > 1080 */
- dpu_cstate->num_mixers = (mode->vdisplay > MAX_VDISPLAY_SPLIT) ? 2
: 1;
- dpu_cstate->num_ctls = dpu_enc->num_phys_encs;
dpu_cstate->num_intfs = dpu_enc->num_phys_encs;
}
@@ -623,9 +617,9 @@ static int dpu_encoder_virt_atomic_check(
}
}
- _dpu_encoder_get_topology(dpu_enc, crtc_state, adj_mode);
+ _dpu_encoder_get_topology(dpu_enc, crtc_state);
if (!ret && drm_atomic_crtc_needs_modeset(crtc_state))
- ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, crtc_state);
+ ret = dpu_rm_encoder_reserve(&dpu_kms->rm, drm_enc,
crtc_state);
if (!ret)
drm_mode_set_crtcinfo(adj_mode, 0);
@@ -1186,7 +1180,7 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
- dpu_rm_release(&dpu_kms->rm, drm_enc->crtc->state);
+ dpu_rm_encoder_release(&dpu_kms->rm, drm_enc->crtc->state);
}
static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg
*catalog,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 5304597..901b1fc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -436,8 +436,8 @@ static int _dpu_rm_release_hw(struct dpu_rm *rm,
enum dpu_hw_blk_type type,
return -EINVAL;
}
-static void _dpu_rm_release_reservation(struct dpu_rm *rm,
- struct dpu_crtc_state *dpu_cstate)
+static void _dpu_rm_crtc_release_reservation(struct dpu_rm *rm,
+ struct dpu_crtc_state
*dpu_cstate)
{
int i;
@@ -464,6 +464,12 @@ static void _dpu_rm_release_reservation(struct
dpu_rm *rm,
dpu_cstate->hw_ctls[i]->base.id))
dpu_cstate->hw_ctls[i] = NULL;
}
+}
+
+static void _dpu_rm_encoder_release_reservation(struct dpu_rm *rm,
+ struct dpu_crtc_state
*dpu_cstate)
+{
+ int i;
for (i = 0; i < dpu_cstate->num_intfs; i++) {
if (!dpu_cstate->hw_intfs[i])
@@ -475,23 +481,33 @@ static void _dpu_rm_release_reservation(struct
dpu_rm *rm,
}
}
-void dpu_rm_release(struct dpu_rm *rm, struct drm_crtc_state
*crtc_state)
+void dpu_rm_crtc_release(struct dpu_rm *rm, struct drm_crtc_state
*crtc_state)
{
struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
mutex_lock(&rm->rm_lock);
- _dpu_rm_release_reservation(rm, dpu_cstate);
+ _dpu_rm_crtc_release_reservation(rm, dpu_cstate);
mutex_unlock(&rm->rm_lock);
}
-int dpu_rm_reserve(
+void dpu_rm_encoder_release(struct dpu_rm *rm,
+ struct drm_crtc_state *crtc_state)
+{
+ struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
+
+ mutex_lock(&rm->rm_lock);
+
+ _dpu_rm_encoder_release_reservation(rm, dpu_cstate);
+
+ mutex_unlock(&rm->rm_lock);
+}
+
+int dpu_rm_crtc_reserve(
struct dpu_rm *rm,
- struct drm_encoder *enc,
struct drm_crtc_state *crtc_state)
{
- struct dpu_encoder_hw_resources hw_res;
struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
int ret;
@@ -499,12 +515,10 @@ int dpu_rm_reserve(
if (!drm_atomic_crtc_needs_modeset(crtc_state))
return 0;
- DRM_DEBUG_KMS("reserving hw for enc %d crtc %d\n",
- enc->base.id, crtc_state->crtc->base.id);
- mutex_lock(&rm->rm_lock);
+ DRM_DEBUG_KMS("reserving hw for crtc %d\n",
crtc_state->crtc->base.id);
- dpu_encoder_get_hw_resources(enc, &hw_res);
+ mutex_lock(&rm->rm_lock);
ret = _dpu_rm_reserve_lms(rm, dpu_cstate);
if (ret) {
@@ -518,6 +532,37 @@ int dpu_rm_reserve(
goto cleanup_on_fail;
}
+ mutex_unlock(&rm->rm_lock);
+
+ return ret;
+
+cleanup_on_fail:
+ _dpu_rm_crtc_release_reservation(rm, dpu_cstate);
+
+ mutex_unlock(&rm->rm_lock);
+
+ return ret;
+}
+
+int dpu_rm_encoder_reserve(
+ struct dpu_rm *rm,
+ struct drm_encoder *enc,
+ struct drm_crtc_state *crtc_state)
+{
+ struct dpu_encoder_hw_resources hw_res;
+ struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
+ int ret;
+
+ /* Check if this is just a page-flip */
+ if (!drm_atomic_crtc_needs_modeset(crtc_state))
+ return 0;
+
+ DRM_DEBUG_KMS("reserving hw for enc %d\n", enc->base.id);
+
+ mutex_lock(&rm->rm_lock);
+
+ dpu_encoder_get_hw_resources(enc, &hw_res);
+
ret = _dpu_rm_reserve_intfs(rm, dpu_cstate, &hw_res);
if (ret) {
DPU_ERROR("unable to find appropriate INTF\n");
@@ -529,7 +574,7 @@ int dpu_rm_reserve(
return ret;
cleanup_on_fail:
- _dpu_rm_release_reservation(rm, dpu_cstate);
+ _dpu_rm_encoder_release_reservation(rm, dpu_cstate);
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 1626cef..0b1deb0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -53,27 +53,45 @@ int dpu_rm_init(struct dpu_rm *rm,
int dpu_rm_destroy(struct dpu_rm *rm);
/**
- * dpu_rm_reserve - Given a CRTC->Encoder->Connector display chain,
analyze
- * the use connections and user requirements, specified through
related
+ * dpu_rm_encoder_reserve - Given an Encoder, analyze the use
connections
+ * and user requirements, specified through related
* topology control properties, and reserve hardware blocks to that
* display chain.
- * HW blocks can then be accessed through dpu_rm_get_* functions.
- * HW Reservations should be released via dpu_rm_release_hw.
* @rm: DPU Resource Manager handle
* @drm_enc: DRM Encoder handle
* @crtc_state: Proposed Atomic DRM CRTC State handle
* @Return: 0 on Success otherwise -ERROR
*/
-int dpu_rm_reserve(struct dpu_rm *rm,
+int dpu_rm_encoder_reserve(struct dpu_rm *rm,
struct drm_encoder *drm_enc,
struct drm_crtc_state *crtc_state);
/**
- * dpu_rm_release - Given the encoder for the display chain, release
any
- * HW blocks previously reserved for that use case.
+ * dpu_rm_crtc_reserve - Given a CRTC, analyze the use connections
+ * and user requirements, specified through related
+ * topology control properties, and reserve hardware blocks to that
+ * display chain.
* @rm: DPU Resource Manager handle
- * @crtc_state: atomic DRM state handle
+ * @crtc_state: Proposed Atomic DRM CRTC State handle
* @Return: 0 on Success otherwise -ERROR
*/
-void dpu_rm_release(struct dpu_rm *rm, struct drm_crtc_state
*crtc_state);
+int dpu_rm_crtc_reserve(struct dpu_rm *rm,
+ struct drm_crtc_state *crtc_state);
+
+/**
+ * dpu_rm_encoder_release - Given the encoder, release any
+ * HW blocks previously reserved for that use case.
+ * @rm: DPU Resource Manager handle
+ * @crtc_state: Proposed Atomic DRM CRTC State handle
+ */
+void dpu_rm_encoder_release(struct dpu_rm *rm,
+ struct drm_crtc_state *crtc_state);
+
+/**
+ * dpu_rm_crtc_release - Given the crtc, release any
+ * HW blocks previously reserved for that use case.
+ * @rm: DPU Resource Manager handle
+ * @crtc_state: Proposed Atomic DRM CRTC State handle
+ */
+void dpu_rm_crtc_release(struct dpu_rm *rm, struct drm_crtc_state
*crtc_state);
#endif /* __DPU_RM_H__ */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
