Re: [Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer

2023-03-03 Thread Dmitry Baryshkov

On 09/02/2023 01:44, Abhinav Kumar wrote:



On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:

Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 86 ---
  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
  2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void 
_dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)

  }
  }
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+   struct drm_plane *plane,
+   struct dpu_crtc_mixer *mixer,
+   u32 num_mixers,
+   struct dpu_hw_stage_cfg *stage_cfg,
+   enum dpu_stage stage,
+   unsigned int stage_idx,
+   unsigned long *fetch_active,
+   struct dpu_sw_pipe *pipe
+  )
+{
+    uint32_t lm_idx;
+    enum dpu_sspp sspp_idx;
+    struct drm_plane_state *state;
+
+    if (!pipe->sspp)
+    return;
+
+    sspp_idx = pipe->sspp->idx;
+
+    state = plane->state;
+
+    DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+ crtc->base.id,
+ stage,
+ plane->base.id,
+ sspp_idx - SSPP_NONE,
+ state->fb ? state->fb->base.id : -1);
+
+    set_bit(sspp_idx, fetch_active);
+
+    stage_cfg->stage[stage][stage_idx] = sspp_idx;
+    stage_cfg->multirect_index[stage][stage_idx] =
+    pipe->multirect_index;
+
+    /* blend config update */
+    for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,

+    sspp_idx);


If you just pass the format to this function you can move rest of the 
for loop also to this function.


As a second thought, this would defeat the purpose of the split. We 
don't have to call _dpu_crtc_setup_blend_cfg() or setup mixer_op_mode 
for the second pipe separately. So, I'd leave the loops as is.




Also, you will be able to add the trace_dpu_crtc_setup_mixer() with 
complete information.


trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is 
important to debug blend issues.



+}
+
  static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
  struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
  struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  struct dpu_format *format;
  struct dpu_hw_ctl *ctl = mixer->lm_ctl;
-    uint32_t stage_idx, lm_idx;
-    int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+    uint32_t lm_idx;
  bool bg_alpha_enable = false;
  DECLARE_BITMAP(fetch_active, SSPP_MAX);
  memset(fetch_active, 0, sizeof(fetch_active));
  drm_atomic_crtc_for_each_plane(plane, crtc) {
-    enum dpu_sspp sspp_idx;
-
  state = plane->state;
  if (!state)
  continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  pstate = to_dpu_plane_state(state);
  fb = state->fb;
-    sspp_idx = pstate->pipe.sspp->idx;
-    set_bit(sspp_idx, fetch_active);
-
-    DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
-    crtc->base.id,
-    pstate->stage,
-    plane->base.id,
-    sspp_idx - SSPP_VIG0,
-    state->fb ? state->fb->base.id : -1);
-
  format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));

  if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
  bg_alpha_enable = true;
-    stage_idx = zpos_cnt[pstate->stage]++;
-    stage_cfg->stage[pstate->stage][stage_idx] =
-    sspp_idx;
-    stage_cfg->multirect_index[pstate->stage][stage_idx] =
-    pstate->pipe.multirect_index;
-
  trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
-   state, pstate, stage_idx,
+   state, pstate,
 format->base.pixel_format,
 fb ? fb->modifier : 0);
+    _dpu_crtc_blend_setup_pipe(crtc, plane,
+   mixer, cstate->num_mixers,
+   stage_cfg, pstate->stage, 0,
+   fetch_active,
+   >pipe);
+
  /* blend config update */
  for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
-    _dpu_crtc_setup_blend_cfg(mixer + lm_idx,
-    pstate, format);
-
- 

Re: [Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer

2023-02-08 Thread Dmitry Baryshkov

On 09/02/2023 01:44, Abhinav Kumar wrote:



On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:

Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 86 ---
  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
  2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void 
_dpu_crtc_program_lm_output_roi(struct drm_crtc *crtc)

  }
  }
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+   struct drm_plane *plane,
+   struct dpu_crtc_mixer *mixer,
+   u32 num_mixers,
+   struct dpu_hw_stage_cfg *stage_cfg,
+   enum dpu_stage stage,
+   unsigned int stage_idx,
+   unsigned long *fetch_active,
+   struct dpu_sw_pipe *pipe
+  )
+{
+    uint32_t lm_idx;
+    enum dpu_sspp sspp_idx;
+    struct drm_plane_state *state;
+
+    if (!pipe->sspp)
+    return;
+
+    sspp_idx = pipe->sspp->idx;
+
+    state = plane->state;
+
+    DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+ crtc->base.id,
+ stage,
+ plane->base.id,
+ sspp_idx - SSPP_NONE,
+ state->fb ? state->fb->base.id : -1);
+
+    set_bit(sspp_idx, fetch_active);
+
+    stage_cfg->stage[stage][stage_idx] = sspp_idx;
+    stage_cfg->multirect_index[stage][stage_idx] =
+    pipe->multirect_index;
+
+    /* blend config update */
+    for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,

+    sspp_idx);


If you just pass the format to this function you can move rest of the 
for loop also to this function.


Also, you will be able to add the trace_dpu_crtc_setup_mixer() with 
complete information.


trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is 
important to debug blend issues.


Ack, I'll add it back, thanks for the note.




+}
+
  static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
  struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
  struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  struct dpu_format *format;
  struct dpu_hw_ctl *ctl = mixer->lm_ctl;
-    uint32_t stage_idx, lm_idx;
-    int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+    uint32_t lm_idx;
  bool bg_alpha_enable = false;
  DECLARE_BITMAP(fetch_active, SSPP_MAX);
  memset(fetch_active, 0, sizeof(fetch_active));
  drm_atomic_crtc_for_each_plane(plane, crtc) {
-    enum dpu_sspp sspp_idx;
-
  state = plane->state;
  if (!state)
  continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct 
drm_crtc *crtc,

  pstate = to_dpu_plane_state(state);
  fb = state->fb;
-    sspp_idx = pstate->pipe.sspp->idx;
-    set_bit(sspp_idx, fetch_active);
-
-    DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
-    crtc->base.id,
-    pstate->stage,
-    plane->base.id,
-    sspp_idx - SSPP_VIG0,
-    state->fb ? state->fb->base.id : -1);
-
  format = 
to_dpu_format(msm_framebuffer_format(pstate->base.fb));

  if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
  bg_alpha_enable = true;
-    stage_idx = zpos_cnt[pstate->stage]++;
-    stage_cfg->stage[pstate->stage][stage_idx] =
-    sspp_idx;
-    stage_cfg->multirect_index[pstate->stage][stage_idx] =
-    pstate->pipe.multirect_index;
-
  trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
-   state, pstate, stage_idx,
+   state, pstate,
 format->base.pixel_format,
 fb ? fb->modifier : 0);
+    _dpu_crtc_blend_setup_pipe(crtc, plane,
+   mixer, cstate->num_mixers,
+   stage_cfg, pstate->stage, 0,
+   fetch_active,
+   >pipe);
+
  /* blend config update */
  for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
-    _dpu_crtc_setup_blend_cfg(mixer + lm_idx,
-    pstate, format);
-
-
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,

-    sspp_idx);
+    

Re: [Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer

2023-02-08 Thread Abhinav Kumar




On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:

Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 86 ---
  drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
  2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void _dpu_crtc_program_lm_output_roi(struct 
drm_crtc *crtc)
}
  }
  
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,

+  struct drm_plane *plane,
+  struct dpu_crtc_mixer *mixer,
+  u32 num_mixers,
+  struct dpu_hw_stage_cfg *stage_cfg,
+  enum dpu_stage stage,
+  unsigned int stage_idx,
+  unsigned long *fetch_active,
+  struct dpu_sw_pipe *pipe
+ )
+{
+   uint32_t lm_idx;
+   enum dpu_sspp sspp_idx;
+   struct drm_plane_state *state;
+
+   if (!pipe->sspp)
+   return;
+
+   sspp_idx = pipe->sspp->idx;
+
+   state = plane->state;
+
+   DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+crtc->base.id,
+stage,
+plane->base.id,
+sspp_idx - SSPP_NONE,
+state->fb ? state->fb->base.id : -1);
+
+   set_bit(sspp_idx, fetch_active);
+
+   stage_cfg->stage[stage][stage_idx] = sspp_idx;
+   stage_cfg->multirect_index[stage][stage_idx] =
+   pipe->multirect_index;
+
+   /* blend config update */
+   for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+   
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
+   sspp_idx);


If you just pass the format to this function you can move rest of the 
for loop also to this function.


Also, you will be able to add the trace_dpu_crtc_setup_mixer() with 
complete information.


trace_dpu_crtc_setup_mixer is currently missing te stage_idx which is 
important to debug blend issues.



+}
+
  static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
  
-	uint32_t stage_idx, lm_idx;

-   int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+   uint32_t lm_idx;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
  
  	memset(fetch_active, 0, sizeof(fetch_active));

drm_atomic_crtc_for_each_plane(plane, crtc) {
-   enum dpu_sspp sspp_idx;
-
state = plane->state;
if (!state)
continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
  
-		sspp_idx = pstate->pipe.sspp->idx;

-   set_bit(sspp_idx, fetch_active);
-
-   DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
-   crtc->base.id,
-   pstate->stage,
-   plane->base.id,
-   sspp_idx - SSPP_VIG0,
-   state->fb ? state->fb->base.id : -1);
-
format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
  
  		if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)

bg_alpha_enable = true;
  
-		stage_idx = zpos_cnt[pstate->stage]++;

-   stage_cfg->stage[pstate->stage][stage_idx] =
-   sspp_idx;
-   stage_cfg->multirect_index[pstate->stage][stage_idx] =
-   pstate->pipe.multirect_index;
-
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
-  state, pstate, stage_idx,
+  state, pstate,
   format->base.pixel_format,
   fb ? fb->modifier : 0);
  
+		_dpu_crtc_blend_setup_pipe(crtc, plane,

+  mixer, 

[Freedreno] [PATCH v3 26/27] drm/msm/dpu: split pipe handling from _dpu_crtc_blend_setup_mixer

2023-02-03 Thread Dmitry Baryshkov
Rework _dpu_crtc_blend_setup_mixer() to split away pipe handling to a
separate functon. This is a preparation for the r_pipe support.

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  | 86 ---
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 10 ++-
 2 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 73e1a8c69ef0..0ca3bc38ff7e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -400,6 +400,47 @@ static void _dpu_crtc_program_lm_output_roi(struct 
drm_crtc *crtc)
}
 }
 
+static void _dpu_crtc_blend_setup_pipe(struct drm_crtc *crtc,
+  struct drm_plane *plane,
+  struct dpu_crtc_mixer *mixer,
+  u32 num_mixers,
+  struct dpu_hw_stage_cfg *stage_cfg,
+  enum dpu_stage stage,
+  unsigned int stage_idx,
+  unsigned long *fetch_active,
+  struct dpu_sw_pipe *pipe
+ )
+{
+   uint32_t lm_idx;
+   enum dpu_sspp sspp_idx;
+   struct drm_plane_state *state;
+
+   if (!pipe->sspp)
+   return;
+
+   sspp_idx = pipe->sspp->idx;
+
+   state = plane->state;
+
+   DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
+crtc->base.id,
+stage,
+plane->base.id,
+sspp_idx - SSPP_NONE,
+state->fb ? state->fb->base.id : -1);
+
+   set_bit(sspp_idx, fetch_active);
+
+   stage_cfg->stage[stage][stage_idx] = sspp_idx;
+   stage_cfg->multirect_index[stage][stage_idx] =
+   pipe->multirect_index;
+
+   /* blend config update */
+   for (lm_idx = 0; lm_idx < num_mixers; lm_idx++)
+   
mixer[lm_idx].lm_ctl->ops.update_pending_flush_sspp(mixer[lm_idx].lm_ctl,
+   sspp_idx);
+}
+
 static void _dpu_crtc_blend_setup_mixer(struct drm_crtc *crtc,
struct dpu_crtc *dpu_crtc, struct dpu_crtc_mixer *mixer,
struct dpu_hw_stage_cfg *stage_cfg)
@@ -412,15 +453,12 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
struct dpu_format *format;
struct dpu_hw_ctl *ctl = mixer->lm_ctl;
 
-   uint32_t stage_idx, lm_idx;
-   int zpos_cnt[DPU_STAGE_MAX + 1] = { 0 };
+   uint32_t lm_idx;
bool bg_alpha_enable = false;
DECLARE_BITMAP(fetch_active, SSPP_MAX);
 
memset(fetch_active, 0, sizeof(fetch_active));
drm_atomic_crtc_for_each_plane(plane, crtc) {
-   enum dpu_sspp sspp_idx;
-
state = plane->state;
if (!state)
continue;
@@ -431,39 +469,25 @@ static void _dpu_crtc_blend_setup_mixer(struct drm_crtc 
*crtc,
pstate = to_dpu_plane_state(state);
fb = state->fb;
 
-   sspp_idx = pstate->pipe.sspp->idx;
-   set_bit(sspp_idx, fetch_active);
-
-   DRM_DEBUG_ATOMIC("crtc %d stage:%d - plane %d sspp %d fb %d\n",
-   crtc->base.id,
-   pstate->stage,
-   plane->base.id,
-   sspp_idx - SSPP_VIG0,
-   state->fb ? state->fb->base.id : -1);
-
format = to_dpu_format(msm_framebuffer_format(pstate->base.fb));
 
if (pstate->stage == DPU_STAGE_BASE && format->alpha_enable)
bg_alpha_enable = true;
 
-   stage_idx = zpos_cnt[pstate->stage]++;
-   stage_cfg->stage[pstate->stage][stage_idx] =
-   sspp_idx;
-   stage_cfg->multirect_index[pstate->stage][stage_idx] =
-   pstate->pipe.multirect_index;
-
trace_dpu_crtc_setup_mixer(DRMID(crtc), DRMID(plane),
-  state, pstate, stage_idx,
+  state, pstate,
   format->base.pixel_format,
   fb ? fb->modifier : 0);
 
+   _dpu_crtc_blend_setup_pipe(crtc, plane,
+  mixer, cstate->num_mixers,
+  stage_cfg, pstate->stage, 0,
+  fetch_active,
+  >pipe);
+
/* blend config update */
for (lm_idx = 0; lm_idx < cstate->num_mixers; lm_idx++) {
-