On Mon, Nov 19, 2018 at 12:03:53PM -0800, Jeykumar Sankaran wrote:
On 2018-11-16 13:14, Sean Paul wrote:
On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
On 2018-11-16 10:42, Sean Paul wrote:
From: Sean Paul seanpaul@chromium.org
It's for legacy drivers, for atomic drivers crtc->state->encoder_mask should be used to map encoder to crtc.
Changes in v2:
- None
Signed-off-by: Sean Paul seanpaul@chromium.org
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
++++++++----------------
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++++++--- 2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 156f4c77ca44..a008a87a8113 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc *crtc) return INTF_MODE_NONE; }
- drm_for_each_encoder(encoder, crtc->dev)
if (encoder->crtc == crtc)return dpu_encoder_get_intf_mode(encoder);
- /* TODO: Returns the first INTF_MODE, could there be multiple
values? */
- drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
return dpu_encoder_get_intf_mode(encoder);return INTF_MODE_NONE;
} @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc, spin_unlock_irqrestore(&dev->event_lock, flags); }
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
{
if (encoder->crtc != crtc)continue;/* encoder will trigger pending mask now */
- /* encoder will trigger pending mask now */
- drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask) dpu_encoder_trigger_kickoff_pending(encoder);
}
/*
- If no mixers have been allocated in dpu_crtc_atomic_check(),
@@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc *crtc) void dpu_crtc_commit_kickoff(struct drm_crtc *crtc) { struct drm_encoder *encoder;
- struct drm_device *dev = crtc->dev; struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc); struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc); struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
@@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
DPU_ATRACE_BEGIN("crtc_commit");
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
{
- /*
* Encoder will flush/start now, unless it has a tx pending. Ifso, it
* may delay and flush at an irq event (e.g. ppdone)*/- drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask) {
if (encoder->crtc != crtc)continue;/** Encoder will flush/start now, unless it has a txpending.
* If so, it may delay and flush at an irq event (e.g.ppdone)
*/@@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
dpu_vbif_clear_errors(dpu_kms);
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
{
if (encoder->crtc != crtc)continue;
- drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask) dpu_encoder_kickoff(encoder);
- }
We wont be holding the modeset locks here (and in crtc_atomic_begin) in
the
display thread. Is it safe to iterate over encoder_mask?
Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for dpu_crtc_commit_kickoff():
1- dpu_kms_encoder_enable() which is called via the encoder->funcs->enable hook 2- dpu_kms_commit() which is called in the mode_config->funcs->atomic_commit_tail
atomic_commit_tail will be called from the WQ. Do we hold the modeset locks in the WQ? I believe userspace thread will drop the locks at the end of drm_mode_atomic_ioctl.
Accessing state is safe in non-blocking commits due to stalling any subsequent commits until the previous one has completed. Check out the comment at:
https://cgit.freedesktop.org/drm-tip/tree/drivers/gpu/drm/drm_atomic_helper....
Sean
Thanks
Both of these callsites will hold the modeset locks.
Am I missing something?
Thanks for your review,
Sean
end: reinit_completion(&dpu_crtc->frame_done_comp); @@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
dpu_core_perf_crtc_update(crtc, 0, true);
- drm_for_each_encoder(encoder, crtc->dev) {
if (encoder->crtc != crtc)continue;
- drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask) dpu_encoder_register_frame_event_callback(encoder, NULL, NULL);
}
memset(cstate->mixers, 0, sizeof(cstate->mixers)); cstate->num_mixers = 0;
@@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc
*crtc,
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id); dpu_crtc = to_dpu_crtc(crtc);
- drm_for_each_encoder(encoder, crtc->dev) {
if (encoder->crtc != crtc)continue;
- drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask) dpu_encoder_register_frame_event_callback(encoder, dpu_crtc_frame_event_cb, (void *)crtc);
}
mutex_lock(&dpu_crtc->crtc_lock); trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 1bec4540f3e1..64134d619748 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -320,7 +320,10 @@ static void dpu_kms_prepare_commit(struct msm_kms *kms, struct dpu_kms *dpu_kms; struct msm_drm_private *priv; struct drm_device *dev;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state; struct drm_encoder *encoder;
int i;
if (!kms) return;
@@ -332,9 +335,13 @@ static void dpu_kms_prepare_commit(struct msm_kms *kms, priv = dev->dev_private; pm_runtime_get_sync(&dpu_kms->pdev->dev);
- list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
if (encoder->crtc != NULL)
- /* Call prepare_commit for all affected encoders */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
drm_for_each_encoder_mask(encoder, crtc->dev,crtc_state->encoder_mask) { dpu_encoder_prepare_commit(encoder);}- }
}
/* @@ -344,13 +351,17 @@ static void dpu_kms_prepare_commit(struct
msm_kms
*kms, void dpu_kms_encoder_enable(struct drm_encoder *encoder) { const struct drm_encoder_helper_funcs *funcs = encoder->helper_private;
- struct drm_crtc *crtc = encoder->crtc;
struct drm_device *dev = encoder->dev;
struct drm_crtc *crtc;
/* Forward this enable call to the commit hook */ if (funcs && funcs->commit) funcs->commit(encoder);
- if (crtc && crtc->state->active) {
- drm_for_each_crtc(crtc, dev) {
if (!(crtc->state->encoder_mask &drm_encoder_mask(encoder)))
continue;- trace_dpu_kms_enc_enable(DRMID(crtc)); dpu_crtc_commit_kickoff(crtc); }
-- Jeykumar S
-- Jeykumar S