}
  +    index = dpu_enc->disp_info.h_tile_instance[0];
+        if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI)
+        dpu_enc->dsc = msm_dsi_get_dsc_config(priv->dsi[index]);

As discussed previously, one should not write to non-state objects from atomic_check. This chunk does.

yes, i did think about not to assign dsc here as we had discussed.

but the get_topology() below did need to know whether dsc is present or not.

otherwise, i have to create a local variable to pass into get_topology() function.

The dsc is assigned here but not yet be used.



Not to mention that this will start exploding once you try adding DP next to it.

Please abstain from posting next revisions until the discussions on the previous one are more or less finished. For now this is NAK.

Not to mention that this patch doesn't pass checkpatch.pl.

+
      topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
        /*
@@ -1034,7 +1038,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
      struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
      int num_lm, num_ctl, num_pp, num_dsc;
      unsigned int dsc_mask = 0;
-    int i;
+    int index, i;
        if (!drm_enc) {
          DPU_ERROR("invalid encoder\n");
@@ -1055,6 +1059,10 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
        trace_dpu_enc_mode_set(DRMID(drm_enc));
  +    index = dpu_enc->disp_info.h_tile_instance[0];
+        if (dpu_enc->disp_info.intf_type == DRM_MODE_ENCODER_DSI)
+        dpu_enc->dsc = msm_dsi_get_dsc_config(priv->dsi[index]);

Doesn't this seem 100% same as the previous chunk? Doesn't it plead to be extracted to a helper function?

+
      /* Query resource that have been reserved in atomic check step. */
      num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
          drm_enc->base.id, DPU_HW_BLK_PINGPONG, hw_pp,
@@ -2121,8 +2129,10 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
                      phys_enc->hw_pp->merge_3d->idx);
      }
  -    if (dpu_enc->dsc)
+    if (dpu_enc->dsc) {
          dpu_encoder_unprep_dsc(dpu_enc);
+        dpu_enc->dsc = NULL;
+    }
        intf_cfg.stream_sel = 0; /* Don't care value for video mode */
      intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);

Reply via email to