I find this title very undescriptive, it doesn't really explain from/to where this move is happening nor why. On 2023-08-02 11:08:48, Jessica Zhang wrote: > Move the setting of dpu_enc.wide_bus_en to > dpu_encoder_virt_atomic_enable() so that it mirrors the setting of > dpu_enc.dsc. mirroring "the setting of dpu_enc.dsc" very much sounds like you are mirroring _its value_, but that is not the case. You are moving the initialization (or just setting, because it could also be overwriting?) to _the same place_ where .dsc is assigned. I am pretty sure that this has a runtime impact which we discussed before (hotplug...?) but the commit message omits that. This is mandatory. - Marijn > > Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index d34e684a4178..3dcd37c48aac 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -1194,11 +1194,18 @@ static void dpu_encoder_virt_atomic_enable(struct drm_encoder *drm_enc, > struct dpu_encoder_virt *dpu_enc = NULL; > int ret = 0; > struct drm_display_mode *cur_mode = NULL; > + struct msm_drm_private *priv = drm_enc->dev->dev_private; > + struct msm_display_info *disp_info; > > dpu_enc = to_dpu_encoder_virt(drm_enc); > + disp_info = &dpu_enc->disp_info; > > dpu_enc->dsc = dpu_encoder_get_dsc_config(drm_enc); > > + if (disp_info->intf_type == INTF_DP) > + dpu_enc->wide_bus_en = msm_dp_wide_bus_available( > + priv->dp[disp_info->h_tile_instance[0]]); > + > mutex_lock(&dpu_enc->enc_lock); > cur_mode = &dpu_enc->base.crtc->state->adjusted_mode; > > @@ -2383,10 +2390,6 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > timer_setup(&dpu_enc->frame_done_timer, > dpu_encoder_frame_done_timeout, 0); > > - if (disp_info->intf_type == INTF_DP) > - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( > - priv->dp[disp_info->h_tile_instance[0]]); > - > INIT_DELAYED_WORK(&dpu_enc->delayed_off_work, > dpu_encoder_off_work); > dpu_enc->idle_timeout = IDLE_TIMEOUT; > > -- > 2.41.0 >