Re: [PATCH 3/7] drm/msm/dpu: separate common function to init physical encoder

2023-05-02 Thread Dmitry Baryshkov 

On 03/05/2023 00:33, Abhinav Kumar wrote:



On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote:

Move common DPU physical encoder initialization code to the new function
dpu_encoder_phys_init().

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 31 +--
  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  3 ++
  .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 19 +++-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 20 +++-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 19 +++-
  5 files changed, 46 insertions(+), 46 deletions(-)

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

index 8c45c949ec39..c60dce5861e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2303,8 +2303,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,

  for (i = 0; i < dpu_enc->num_phys_encs; i++) {
  struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
-    atomic_set(>vsync_cnt, 0);
-    atomic_set(>underrun_cnt, 0);
  if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
  phys->hw_intf = dpu_rm_get_intf(_kms->rm, 
phys->intf_idx);
@@ -2505,3 +2503,32 @@ unsigned int dpu_encoder_helper_get_dsc(struct 
dpu_encoder_phys *phys_enc)

  return dpu_enc->dsc_mask;
  }
+
+int dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc,
+  struct dpu_enc_phys_init_params *p)
+{
+    int i;
+
+    phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
+    phys_enc->intf_idx = p->intf_idx;
+    phys_enc->wb_idx = p->wb_idx;
+    phys_enc->parent = p->parent;
+    phys_enc->dpu_kms = p->dpu_kms;
+    phys_enc->split_role = p->split_role;
+    phys_enc->enc_spinlock = p->enc_spinlock;
+    phys_enc->enable_state = DPU_ENC_DISABLED;
+
+    for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
+    phys_enc->irq[i] = -EINVAL;
+
+    atomic_set(_enc->vblank_refcount, 0);
+    atomic_set(_enc->pending_kickoff_cnt, 0);
+    atomic_set(_enc->pending_ctlstart_cnt, 0);
+
+    atomic_set(_enc->vsync_cnt, 0);
+    atomic_set(_enc->underrun_cnt, 0);
+
+    init_waitqueue_head(_enc->pending_kickoff_wq);
+
+    return 0;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h

index 1d434b22180d..7019870215c0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -405,4 +405,7 @@ void dpu_encoder_frame_done_callback(
  struct drm_encoder *drm_enc,
  struct dpu_encoder_phys *ready_phys, u32 event);
+int dpu_encoder_phys_init(struct dpu_encoder_phys *phys,
+  struct dpu_enc_phys_init_params *p);
+
  #endif /* __dpu_encoder_phys_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c

index 74470d068622..ce86b9ef6bf1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -759,7 +759,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
  {
  struct dpu_encoder_phys *phys_enc = NULL;
  struct dpu_encoder_phys_cmd *cmd_enc = NULL;
-    int i, ret = 0;
+    int ret = 0;
  DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
@@ -770,25 +770,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
  return ERR_PTR(ret);
  }
  phys_enc = _enc->base;
-    phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
-    phys_enc->intf_idx = p->intf_idx;
+
+    ret = dpu_encoder_phys_init(phys_enc, p);
+    if (ret)
+    return ERR_PTR(ret);


dpu_encoder_phys_init() seems to always return 0, so we can make that 
void and drop ret and return here?


I had in mind a possible error from INTF_n/WB_n -> hw_intf/hw_wb lookup, 
but at the end I got rid of that. So, yes, why not.





  dpu_encoder_phys_cmd_init_ops(_enc->ops);
-    phys_enc->parent = p->parent;
-    phys_enc->dpu_kms = p->dpu_kms;
-    phys_enc->split_role = p->split_role;
  phys_enc->intf_mode = INTF_MODE_CMD;
-    phys_enc->enc_spinlock = p->enc_spinlock;
  cmd_enc->stream_sel = 0;
-    phys_enc->enable_state = DPU_ENC_DISABLED;
-    for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
-    phys_enc->irq[i] = -EINVAL;
-    atomic_set(_enc->vblank_refcount, 0);
-    atomic_set(_enc->pending_kickoff_cnt, 0);
-    atomic_set(_enc->pending_ctlstart_cnt, 0);
  atomic_set(_enc->pending_vblank_cnt, 0);
-    init_waitqueue_head(_enc->pending_kickoff_wq);
  init_waitqueue_head(_enc->pending_vblank_wq);
  DPU_DEBUG_CMDENC(cmd_enc, "created\n");
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c

index 3a374292f311..aca3849621e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -699,7

Re: [PATCH 3/7] drm/msm/dpu: separate common function to init physical encoder

2023-05-02 Thread Abhinav Kumar 




On 4/30/2023 4:57 PM, Dmitry Baryshkov wrote:

Move common DPU physical encoder initialization code to the new function
dpu_encoder_phys_init().

Signed-off-by: Dmitry Baryshkov 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 31 +--
  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  3 ++
  .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 19 +++-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 20 +++-
  .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 19 +++-
  5 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8c45c949ec39..c60dce5861e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2303,8 +2303,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
  
  	for (i = 0; i < dpu_enc->num_phys_encs; i++) {

struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
-   atomic_set(>vsync_cnt, 0);
-   atomic_set(>underrun_cnt, 0);
  
  		if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)

phys->hw_intf = dpu_rm_get_intf(_kms->rm, 
phys->intf_idx);
@@ -2505,3 +2503,32 @@ unsigned int dpu_encoder_helper_get_dsc(struct 
dpu_encoder_phys *phys_enc)
  
  	return dpu_enc->dsc_mask;

  }
+
+int dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc,
+ struct dpu_enc_phys_init_params *p)
+{
+   int i;
+
+   phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
+   phys_enc->intf_idx = p->intf_idx;
+   phys_enc->wb_idx = p->wb_idx;
+   phys_enc->parent = p->parent;
+   phys_enc->dpu_kms = p->dpu_kms;
+   phys_enc->split_role = p->split_role;
+   phys_enc->enc_spinlock = p->enc_spinlock;
+   phys_enc->enable_state = DPU_ENC_DISABLED;
+
+   for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
+   phys_enc->irq[i] = -EINVAL;
+
+   atomic_set(_enc->vblank_refcount, 0);
+   atomic_set(_enc->pending_kickoff_cnt, 0);
+   atomic_set(_enc->pending_ctlstart_cnt, 0);
+
+   atomic_set(_enc->vsync_cnt, 0);
+   atomic_set(_enc->underrun_cnt, 0);
+
+   init_waitqueue_head(_enc->pending_kickoff_wq);
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 1d434b22180d..7019870215c0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -405,4 +405,7 @@ void dpu_encoder_frame_done_callback(
struct drm_encoder *drm_enc,
struct dpu_encoder_phys *ready_phys, u32 event);
  
+int dpu_encoder_phys_init(struct dpu_encoder_phys *phys,

+ struct dpu_enc_phys_init_params *p);
+
  #endif /* __dpu_encoder_phys_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 74470d068622..ce86b9ef6bf1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -759,7 +759,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
  {
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_cmd *cmd_enc = NULL;
-   int i, ret = 0;
+   int ret = 0;
  
  	DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
  
@@ -770,25 +770,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(

return ERR_PTR(ret);
}
phys_enc = _enc->base;
-   phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
-   phys_enc->intf_idx = p->intf_idx;
+
+   ret = dpu_encoder_phys_init(phys_enc, p);
+   if (ret)
+   return ERR_PTR(ret);


dpu_encoder_phys_init() seems to always return 0, so we can make that 
void and drop ret and return here?


  
  	dpu_encoder_phys_cmd_init_ops(_enc->ops);

-   phys_enc->parent = p->parent;
-   phys_enc->dpu_kms = p->dpu_kms;
-   phys_enc->split_role = p->split_role;
phys_enc->intf_mode = INTF_MODE_CMD;
-   phys_enc->enc_spinlock = p->enc_spinlock;
cmd_enc->stream_sel = 0;
-   phys_enc->enable_state = DPU_ENC_DISABLED;
-   for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
-   phys_enc->irq[i] = -EINVAL;
  
-	atomic_set(_enc->vblank_refcount, 0);

-   atomic_set(_enc->pending_kickoff_cnt, 0);
-   atomic_set(_enc->pending_ctlstart_cnt, 0);
atomic_set(_enc->pending_vblank_cnt, 0);
-   init_waitqueue_head(_enc->pending_kickoff_wq);
init_waitqueue_head(_enc->pending_vblank_wq);
  
  	DPU_DEBUG_CMDENC(cmd_enc, "created\n");

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 3a374292f311..aca3849621e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++

[PATCH 3/7] drm/msm/dpu: separate common function to init physical encoder

2023-04-30 Thread Dmitry Baryshkov 
Move common DPU physical encoder initialization code to the new function
dpu_encoder_phys_init().

Signed-off-by: Dmitry Baryshkov 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 31 +--
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  3 ++
 .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c  | 19 +++-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 20 +++-
 .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 19 +++-
 5 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8c45c949ec39..c60dce5861e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2303,8 +2303,6 @@ static int dpu_encoder_setup_display(struct 
dpu_encoder_virt *dpu_enc,
 
for (i = 0; i < dpu_enc->num_phys_encs; i++) {
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
-   atomic_set(>vsync_cnt, 0);
-   atomic_set(>underrun_cnt, 0);
 
if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
phys->hw_intf = dpu_rm_get_intf(_kms->rm, 
phys->intf_idx);
@@ -2505,3 +2503,32 @@ unsigned int dpu_encoder_helper_get_dsc(struct 
dpu_encoder_phys *phys_enc)
 
return dpu_enc->dsc_mask;
 }
+
+int dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc,
+ struct dpu_enc_phys_init_params *p)
+{
+   int i;
+
+   phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
+   phys_enc->intf_idx = p->intf_idx;
+   phys_enc->wb_idx = p->wb_idx;
+   phys_enc->parent = p->parent;
+   phys_enc->dpu_kms = p->dpu_kms;
+   phys_enc->split_role = p->split_role;
+   phys_enc->enc_spinlock = p->enc_spinlock;
+   phys_enc->enable_state = DPU_ENC_DISABLED;
+
+   for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
+   phys_enc->irq[i] = -EINVAL;
+
+   atomic_set(_enc->vblank_refcount, 0);
+   atomic_set(_enc->pending_kickoff_cnt, 0);
+   atomic_set(_enc->pending_ctlstart_cnt, 0);
+
+   atomic_set(_enc->vsync_cnt, 0);
+   atomic_set(_enc->underrun_cnt, 0);
+
+   init_waitqueue_head(_enc->pending_kickoff_wq);
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 1d434b22180d..7019870215c0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -405,4 +405,7 @@ void dpu_encoder_frame_done_callback(
struct drm_encoder *drm_enc,
struct dpu_encoder_phys *ready_phys, u32 event);
 
+int dpu_encoder_phys_init(struct dpu_encoder_phys *phys,
+ struct dpu_enc_phys_init_params *p);
+
 #endif /* __dpu_encoder_phys_H__ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 74470d068622..ce86b9ef6bf1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -759,7 +759,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
 {
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_cmd *cmd_enc = NULL;
-   int i, ret = 0;
+   int ret = 0;
 
DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
 
@@ -770,25 +770,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
return ERR_PTR(ret);
}
phys_enc = _enc->base;
-   phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
-   phys_enc->intf_idx = p->intf_idx;
+
+   ret = dpu_encoder_phys_init(phys_enc, p);
+   if (ret)
+   return ERR_PTR(ret);
 
dpu_encoder_phys_cmd_init_ops(_enc->ops);
-   phys_enc->parent = p->parent;
-   phys_enc->dpu_kms = p->dpu_kms;
-   phys_enc->split_role = p->split_role;
phys_enc->intf_mode = INTF_MODE_CMD;
-   phys_enc->enc_spinlock = p->enc_spinlock;
cmd_enc->stream_sel = 0;
-   phys_enc->enable_state = DPU_ENC_DISABLED;
-   for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
-   phys_enc->irq[i] = -EINVAL;
 
-   atomic_set(_enc->vblank_refcount, 0);
-   atomic_set(_enc->pending_kickoff_cnt, 0);
-   atomic_set(_enc->pending_ctlstart_cnt, 0);
atomic_set(_enc->pending_vblank_cnt, 0);
-   init_waitqueue_head(_enc->pending_kickoff_wq);
init_waitqueue_head(_enc->pending_vblank_wq);
 
DPU_DEBUG_CMDENC(cmd_enc, "created\n");
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 3a374292f311..aca3849621e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -699,7 +699,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
struct dpu_enc_phys_init_params