On Tue, Aug 07, 2018 at 08:12:34PM -0700, Jeykumar Sankaran wrote:
hw intf blocks are needed only during encoder enable to program timing engines(for video panels). encoder->enable is triggered only after atomic_modeset at which point we assign the resources for the display pipeline. This patch defers the hw_intf look-up until encoder enable.
I'm sure there's a good reason for this, but you haven't stated it. As a casual reader of the patch, I'd be inclined to prefer doing work only once at init vs everytime we enable.
changes in v2:
- none
changes in v3:
- none
Change-Id: Ib0a2253431468151355e50cbad7b91e2b77b6e54 Signed-off-by: Jeykumar Sankaran jsanka@codeaurora.org
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 53 +++++++--------------- 1 file changed, 16 insertions(+), 37 deletions(-)
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 c0221cc..a0b3744 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 @@ -462,7 +462,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) { struct msm_drm_private *priv; struct dpu_encoder_phys_vid *vid_enc;
- struct dpu_hw_intf *intf;
- struct dpu_rm_hw_iter iter; struct dpu_hw_ctl *ctl; u32 flush_mask = 0;
@@ -474,11 +474,20 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) priv = phys_enc->parent->dev->dev_private;
vid_enc = to_dpu_encoder_phys_vid(phys_enc);
- intf = vid_enc->hw_intf; ctl = phys_enc->hw_ctl;
- if (!vid_enc->hw_intf || !phys_enc->hw_ctl) {
DPU_ERROR("invalid hw_intf %d hw_ctl %d\n",vid_enc->hw_intf != 0, phys_enc->hw_ctl != 0);
- dpu_rm_init_hw_iter(&iter, phys_enc->parent->base.id, DPU_HW_BLK_INTF);
- while (dpu_rm_get_hw(&phys_enc->dpu_kms->rm, &iter)) {
struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;if (hw_intf->idx == phys_enc->intf_idx) {vid_enc->hw_intf = hw_intf;break;}- }
- if (!vid_enc->hw_intf) {
DPU_ERROR("hw_intf not assigned\n");@@ -500,7 +509,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) !dpu_encoder_phys_vid_is_master(phys_enc)) goto skip_flush;
- ctl->ops.get_bitmask_intf(ctl, &flush_mask, intf->idx);
- ctl->ops.get_bitmask_intf(ctl, &flush_mask, vid_enc->hw_intf->idx); ctl->ops.update_pending_flush(ctl, flush_mask);
skip_flush: @@ -531,22 +540,13 @@ static void dpu_encoder_phys_vid_get_hw_resources( struct dpu_encoder_hw_resources *hw_res, struct drm_connector_state *conn_state) {
struct dpu_encoder_phys_vid *vid_enc;
if (!phys_enc || !hw_res) { DPU_ERROR("invalid arg(s), enc %d hw_res %d conn_state %d\n", phys_enc != 0, hw_res != 0, conn_state != 0); return; }
vid_enc = to_dpu_encoder_phys_vid(phys_enc);
if (!vid_enc->hw_intf) {
DPU_ERROR("invalid arg(s), hw_intf\n");return;}
DPU_DEBUG_VIDENC(vid_enc, "\n");
hw_res->intfs[vid_enc->hw_intf->idx - INTF_0] = INTF_MODE_VIDEO;
- hw_res->intfs[phys_enc->intf_idx - INTF_0] = INTF_MODE_VIDEO;
}
static int _dpu_encoder_phys_vid_wait_for_vblank( @@ -809,7 +809,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init( { struct dpu_encoder_phys *phys_enc = NULL; struct dpu_encoder_phys_vid *vid_enc = NULL;
- struct dpu_rm_hw_iter iter; struct dpu_encoder_irq *irq; int i, ret = 0;
@@ -829,26 +828,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init( phys_enc->hw_mdptop = p->dpu_kms->hw_mdp; phys_enc->intf_idx = p->intf_idx;
/**
* hw_intf resource permanently assigned to this encoder* Other resources allocated at atomic commit time by use case*/dpu_rm_init_hw_iter(&iter, 0, DPU_HW_BLK_INTF);
while (dpu_rm_get_hw(&p->dpu_kms->rm, &iter)) {
struct dpu_hw_intf *hw_intf = (struct dpu_hw_intf *)iter.hw;if (hw_intf->idx == p->intf_idx) {vid_enc->hw_intf = hw_intf;break;}}
if (!vid_enc->hw_intf) {
ret = -EINVAL;DPU_ERROR("failed to get hw_intf\n");goto fail;}
DPU_DEBUG_VIDENC(vid_enc, "\n");
dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project