Re: [PATCH v3 22/27] drm/msm/dpu: rework dpu_plane_sspp_atomic_update()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/02/2023 02:22, Abhinav Kumar wrote:


On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Split pipe-dependent code from dpu_plane_sspp_atomic_update() into the

sspp-dependent?

separate function dpu_plane_sspp_update_pipe(). This is one of
preparational steps to add r_pipe support.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

Just a couple of minor comments below but otherwise this split up lgtm

---
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 113 ++++++++++++----------
  1 file changed, 63 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0986e740b978..f94e132733f3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -404,12 +404,13 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
   * _dpu_plane_set_ot_limit - set OT limit for the given plane
   * @plane:        Pointer to drm plane
   * @pipe:        Pointer to software pipe
- * @crtc:        Pointer to drm crtc
   * @pipe_cfg:        Pointer to pipe configuration
+ * @frame_rate:        CRTC's frame rate

Can you please check the spacing here. There seems to be an extra tab before the CRTC's frame rate

I checked, the ident is correct here. It uses tabs, maybe that confuses your mailer.


   */
  static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
          struct dpu_sw_pipe *pipe,
-        struct drm_crtc *crtc, struct dpu_hw_sspp_cfg *pipe_cfg)
+        struct dpu_hw_sspp_cfg *pipe_cfg,
+        int frame_rate)
  {
      struct dpu_plane *pdpu = to_dpu_plane(plane);
      struct dpu_vbif_set_ot_params ot_params;
@@ -421,7 +422,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
      ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
      ot_params.height = drm_rect_height(&pipe_cfg->src_rect);
      ot_params.is_wfd = !pdpu->is_rt_pipe;
-    ot_params.frame_rate = drm_mode_vrefresh(&crtc->mode);
+    ot_params.frame_rate = frame_rate;
      ot_params.vbif_idx = VBIF_RT;
      ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
      ot_params.rd = true;
@@ -457,26 +458,6 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
      dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
  }
-static void _dpu_plane_set_scanout(struct drm_plane *plane,
-        struct dpu_plane_state *pstate,
-        struct drm_framebuffer *fb)
-{
-    struct dpu_plane *pdpu = to_dpu_plane(plane);
-    struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
-    struct msm_gem_address_space *aspace = kms->base.aspace;
-    struct dpu_hw_fmt_layout layout;
-    int ret;
-
-    ret = dpu_format_populate_layout(aspace, fb, &layout);
-    if (ret)
-        DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
-    else if (pstate->pipe.sspp->ops.setup_sourceaddress) {
-        trace_dpu_plane_set_scanout(&pstate->pipe,
-                        &layout);
-        pstate->pipe.sspp->ops.setup_sourceaddress(&pstate->pipe, &layout);
-    }
-}
-
  static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
          uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h,
          struct dpu_hw_scaler3_cfg *scale_cfg,
@@ -1102,35 +1083,25 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error)
      pdpu->is_error = error;
  }
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_update_pipe(struct drm_plane *plane,
+                       struct dpu_sw_pipe *pipe,
+                       struct dpu_hw_sspp_cfg *pipe_cfg,

You can call this parameter sspp_cfg instead of pipe_cfg?

+                       const struct dpu_format *fmt,
+                       int frame_rate,
+                       struct dpu_hw_fmt_layout *layout)
  {
      uint32_t src_flags;
      struct dpu_plane *pdpu = to_dpu_plane(plane);
      struct drm_plane_state *state = plane->state;
      struct dpu_plane_state *pstate = to_dpu_plane_state(state);
-    struct dpu_sw_pipe *pipe = &pstate->pipe;
-    struct drm_crtc *crtc = state->crtc;
-    struct drm_framebuffer *fb = state->fb;
-    bool is_rt_pipe;
-    const struct dpu_format *fmt =
-        to_dpu_format(msm_framebuffer_format(fb));
-    struct dpu_hw_sspp_cfg *pipe_cfg = &pstate->pipe_cfg;
-    _dpu_plane_set_scanout(plane, pstate, fb);
-
-    pstate->pending = true;
-
-    is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
-    pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
-    pdpu->is_rt_pipe = is_rt_pipe;
+    if (layout && pipe->sspp->ops.setup_sourceaddress) {
+        trace_dpu_plane_set_scanout(pipe, layout);
+        pipe->sspp->ops.setup_sourceaddress(pipe, layout);
+    }
      _dpu_plane_set_qos_ctrl(plane, pipe, false, DPU_PLANE_QOS_PANIC_CTRL); -    DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT -            ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src),
-            crtc->base.id, DRM_RECT_ARG(&state->dst),
-            (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
-
      /* override for color fill */
      if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
          /* skip remaining processing on color fill */
@@ -1183,22 +1154,64 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
          }
      }
-    _dpu_plane_set_qos_lut(plane, pipe, fmt, &pstate->pipe_cfg);
+    _dpu_plane_set_qos_lut(plane, pipe, fmt, pipe_cfg);
      _dpu_plane_set_danger_lut(plane, pipe, fmt);
      if (plane->type != DRM_PLANE_TYPE_CURSOR) {
          _dpu_plane_set_qos_ctrl(plane, pipe, true, DPU_PLANE_QOS_PANIC_CTRL);
-        _dpu_plane_set_ot_limit(plane, pipe, crtc, &pstate->pipe_cfg);
+        _dpu_plane_set_ot_limit(plane, pipe, pipe_cfg, frame_rate);
      }
-    if (pstate->needs_qos_remap) {
-        pstate->needs_qos_remap = false;
+    if (pstate->needs_qos_remap)
          _dpu_plane_set_qos_remap(plane, pipe);
-    }
+}
+
+static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+{
+    struct dpu_plane *pdpu = to_dpu_plane(plane);
+    struct drm_plane_state *state = plane->state;
+    struct dpu_plane_state *pstate = to_dpu_plane_state(state);
+    struct dpu_sw_pipe *pipe = &pstate->pipe;
+    struct drm_crtc *crtc = state->crtc;
+    struct drm_framebuffer *fb = state->fb;
+    bool is_rt_pipe;
+    const struct dpu_format *fmt =
+        to_dpu_format(msm_framebuffer_format(fb));
+    struct dpu_hw_sspp_cfg *pipe_cfg = &pstate->pipe_cfg;
+
+    struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
+    struct msm_gem_address_space *aspace = kms->base.aspace;
+    struct dpu_hw_fmt_layout layout;
+    bool layout_valid = false;
+    int ret;
+
+    ret = dpu_format_populate_layout(aspace, fb, &layout);
+    if (ret)
+        DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
+    else
+        layout_valid = true;
+
+    pstate->pending = true;
+
+    is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
+    pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
+    pdpu->is_rt_pipe = is_rt_pipe;
+
+    DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT +            ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src),
+            crtc->base.id, DRM_RECT_ARG(&state->dst),
+            (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt));
+
+    dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
+                   drm_mode_vrefresh(&crtc->mode),
+                   layout_valid ? &layout: NULL);
+
+    if (pstate->needs_qos_remap)
+        pstate->needs_qos_remap = false;
-    pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, &pstate->pipe_cfg); +    pstate->plane_fetch_bw = _dpu_plane_calc_bw(pdpu->catalog, fmt, &crtc->mode, pipe_cfg); -    pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, &pstate->pipe_cfg);
+    pstate->plane_clk = _dpu_plane_calc_clk(&crtc->mode, pipe_cfg);
  }
  static void _dpu_plane_atomic_disable(struct drm_plane *plane)

--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux