Re: [PATCH v3 15/27] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()

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

 



On 04/02/2023 00:44, Abhinav Kumar wrote:


On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 18 +-----------------
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++++++++++--------
  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h |  6 ------
  3 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b485234eefb2..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
                                        crtc);
      struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
      struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
-    struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
      const struct drm_plane_state *pstate;
      struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
      crtc_rect.x2 = mode->hdisplay;
      crtc_rect.y2 = mode->vdisplay;
-     /* get plane state for all drm planes associated with crtc state */
+    /* FIXME: move this to dpu_plane_atomic_check? */
      drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {           struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);
          struct drm_rect dst, clip = crtc_rect;
-        int stage;
          if (IS_ERR_OR_NULL(pstate)) {
              rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
          dpu_pstate->needs_dirtyfb = needs_dirtyfb;
-        dpu_plane_clear_multirect(pstate);
-
          dst = drm_plane_state_dest(pstate);
          if (!drm_rect_intersect(&clip, &dst)) {
              DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
                    DRM_RECT_ARG(&dst));
              return -E2BIG;
          }
-
-        /* verify stage setting before using it */
-        stage = DPU_STAGE_0 + pstate->normalized_zpos;
-        if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
-            DPU_ERROR("> %d plane stages assigned\n",
-                    dpu_kms->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
-            return -EINVAL;
-        }
-
-        to_dpu_plane_state(pstate)->stage = stage;
-        DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
      }
      atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1b3033b15bfa..5aabf9694a53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu,
      return 0;
  }
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
-{
-    struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
-    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
-    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
  int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane)
  {
      struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
      if (!new_plane_state->visible)
          return 0;
+    pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+    pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+

But I am not sure if clearing the multirect belongs here and now I want to clarify one thing about https://patchwork.freedesktop.org/patch/521354/?series=99909&rev=4 which was R-bed in the v1 and carried fwd since then.

So prior to that change, we were only clearing the multirects of the planes that were staged to the crtc and we were getting those from the crtc state. But now we are clearing the multirect of all the planes.

Dont we have to keep that in the crtc_atomic_check() since we do that on all the planes attached to a certain CRTC.

In that case shouldnt we keep this in the crtc_atomic_check() and bring back pipe_staged[] without the multirect and source split cases ofcourse.

What for? In other words, what would be the difference?


+    pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
+    if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
+        DPU_ERROR("> %d plane stages assigned\n",
+                pdpu->catalog->caps->max_mixer_blendstages - DPU_STAGE_0);
+        return -EINVAL;
+    }
+

I agree that this check belongs to the plane_atomic_check().

      src.x1 = new_plane_state->src_x >> 16;
      src.y1 = new_plane_state->src_y >> 16;
      src.x2 = src.x1 + (new_plane_state->src_w >> 16);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index 228db401e905..a08b0539513b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -88,12 +88,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
   */
  int dpu_plane_validate_multirect_v2(struct dpu_multirect_plane_states *plane);
-/**
- * dpu_plane_clear_multirect - clear multirect bits for the given pipe
- * @drm_state: Pointer to DRM plane state
- */
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state);
-
  /**
   * dpu_plane_color_fill - enables color fill on plane
   * @plane:  Pointer to DRM plane object

--
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