* [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
@ 2023-05-19 2:38 Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup() Dmitry Baryshkov
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
Rework dpu_encoder initialization code, simplifying calling sequences
and separating common init parts.
Changes since v1:
- Withdrawn two pathes for a later consideration
- Changed dpu_encoder_phys_init() to return void (Abhinav)
- Added small simplifications of dpu_encoder_phys_cmd_init() and
dpu_encoder_phys_wb_init()
Dmitry Baryshkov (7):
drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
drm/msm/dpu: separate common function to init physical encoder
drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
drm/msm/dpu: inline dpu_encoder_get_wb()
drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178 ++++++++----------
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
7 files changed, 140 insertions(+), 243 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 2/7] drm/msm/dpu: separate common function to init physical encoder Dmitry Baryshkov
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
There is no reason to split the dpu_encoder interface into separate
_init() and _setup() phases. Merge them into a single function.
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 55 +++++--------
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++++++-------------
3 files changed, 56 insertions(+), 100 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index b34416cbd0f5..32785cb1b079 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2377,7 +2377,8 @@ static const struct drm_encoder_funcs dpu_encoder_funcs = {
.early_unregister = dpu_encoder_early_unregister,
};
-int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
+struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
+ int drm_enc_mode,
struct msm_display_info *disp_info)
{
struct msm_drm_private *priv = dev->dev_private;
@@ -2386,7 +2387,23 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
struct dpu_encoder_virt *dpu_enc = NULL;
int ret = 0;
- dpu_enc = to_dpu_encoder_virt(enc);
+ dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
+ if (!dpu_enc)
+ return ERR_PTR(-ENOMEM);
+
+ ret = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
+ drm_enc_mode, NULL);
+ if (ret) {
+ devm_kfree(dev->dev, dpu_enc);
+ return ERR_PTR(ret);
+ }
+
+ drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs);
+
+ spin_lock_init(&dpu_enc->enc_spinlock);
+ dpu_enc->enabled = false;
+ mutex_init(&dpu_enc->enc_lock);
+ mutex_init(&dpu_enc->rc_lock);
ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info);
if (ret)
@@ -2415,44 +2432,14 @@ int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
DPU_DEBUG_ENC(dpu_enc, "created\n");
- return ret;
+ return &dpu_enc->base;
fail:
DPU_ERROR("failed to create encoder\n");
if (drm_enc)
dpu_encoder_destroy(drm_enc);
- return ret;
-
-
-}
-
-struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
- int drm_enc_mode)
-{
- struct dpu_encoder_virt *dpu_enc = NULL;
- int rc = 0;
-
- dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL);
- if (!dpu_enc)
- return ERR_PTR(-ENOMEM);
-
-
- rc = drm_encoder_init(dev, &dpu_enc->base, &dpu_encoder_funcs,
- drm_enc_mode, NULL);
- if (rc) {
- devm_kfree(dev->dev, dpu_enc);
- return ERR_PTR(rc);
- }
-
- drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs);
-
- spin_lock_init(&dpu_enc->enc_spinlock);
- dpu_enc->enabled = false;
- mutex_init(&dpu_enc->enc_lock);
- mutex_init(&dpu_enc->rc_lock);
-
- return &dpu_enc->base;
+ return ERR_PTR(ret);
}
int dpu_encoder_wait_for_event(struct drm_encoder *drm_enc,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 6d14f84dd43f..90e1925d7770 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -130,20 +130,12 @@ void dpu_encoder_virt_runtime_resume(struct drm_encoder *encoder);
/**
* dpu_encoder_init - initialize virtual encoder object
* @dev: Pointer to drm device structure
+ * @drm_enc_mode: corresponding DRM_MODE_ENCODER_* constant
* @disp_info: Pointer to display information structure
* Returns: Pointer to newly created drm encoder
*/
-struct drm_encoder *dpu_encoder_init(
- struct drm_device *dev,
- int drm_enc_mode);
-
-/**
- * dpu_encoder_setup - setup dpu_encoder for the display probed
- * @dev: Pointer to drm device structure
- * @enc: Pointer to the drm_encoder
- * @disp_info: Pointer to the display info
- */
-int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
+struct drm_encoder *dpu_encoder_init(struct drm_device *dev,
+ int drm_enc_mode,
struct msm_display_info *disp_info);
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index e1fd7b0f39cd..10bd0fd4ff48 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -535,15 +535,23 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
!msm_dsi_is_master_dsi(priv->dsi[i]))
continue;
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI);
+ memset(&info, 0, sizeof(info));
+ info.intf_type = INTF_DSI;
+
+ info.h_tile_instance[info.num_of_h_tiles++] = i;
+ if (msm_dsi_is_bonded_dsi(priv->dsi[i]))
+ info.h_tile_instance[info.num_of_h_tiles++] = other;
+
+ info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
+
+ info.dsc = msm_dsi_get_dsc_config(priv->dsi[i]);
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_DSI, &info);
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
return PTR_ERR(encoder);
}
- memset(&info, 0, sizeof(info));
- info.intf_type = INTF_DSI;
-
rc = msm_dsi_modeset_init(priv->dsi[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for dsi[%d], rc = %d\n",
@@ -551,11 +559,6 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
break;
}
- info.h_tile_instance[info.num_of_h_tiles++] = i;
- info.is_cmd_mode = msm_dsi_is_cmd_mode(priv->dsi[i]);
-
- info.dsc = msm_dsi_get_dsc_config(priv->dsi[i]);
-
if (msm_dsi_is_bonded_dsi(priv->dsi[i]) && priv->dsi[other]) {
rc = msm_dsi_modeset_init(priv->dsi[other], dev, encoder);
if (rc) {
@@ -563,14 +566,7 @@ static int _dpu_kms_initialize_dsi(struct drm_device *dev,
other, rc);
break;
}
-
- info.h_tile_instance[info.num_of_h_tiles++] = other;
}
-
- rc = dpu_encoder_setup(dev, encoder, &info);
- if (rc)
- DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
- encoder->base.id, rc);
}
return rc;
@@ -589,29 +585,23 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
if (!priv->dp[i])
continue;
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+ memset(&info, 0, sizeof(info));
+ info.num_of_h_tiles = 1;
+ info.h_tile_instance[0] = i;
+ info.intf_type = INTF_DP;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS, &info);
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
return PTR_ERR(encoder);
}
- memset(&info, 0, sizeof(info));
rc = msm_dp_modeset_init(priv->dp[i], dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
drm_encoder_cleanup(encoder);
return rc;
}
-
- info.num_of_h_tiles = 1;
- info.h_tile_instance[0] = i;
- info.intf_type = INTF_DP;
- rc = dpu_encoder_setup(dev, encoder, &info);
- if (rc) {
- DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
- encoder->base.id, rc);
- return rc;
- }
}
return 0;
@@ -629,13 +619,17 @@ static int _dpu_kms_initialize_hdmi(struct drm_device *dev,
if (!priv->hdmi)
return 0;
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+ memset(&info, 0, sizeof(info));
+ info.num_of_h_tiles = 1;
+ info.h_tile_instance[0] = i;
+ info.intf_type = INTF_HDMI;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS, &info);
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for HDMI display\n");
return PTR_ERR(encoder);
}
- memset(&info, 0, sizeof(info));
rc = msm_hdmi_modeset_init(priv->hdmi, dev, encoder);
if (rc) {
DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
@@ -643,16 +637,6 @@ static int _dpu_kms_initialize_hdmi(struct drm_device *dev,
return rc;
}
- info.num_of_h_tiles = 1;
- info.h_tile_instance[0] = i;
- info.intf_type = INTF_HDMI;
- rc = dpu_encoder_setup(dev, encoder, &info);
- if (rc) {
- DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
- encoder->base.id, rc);
- return rc;
- }
-
return 0;
}
@@ -664,14 +648,19 @@ static int _dpu_kms_initialize_writeback(struct drm_device *dev,
struct msm_display_info info;
int rc;
- encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_VIRTUAL);
+ memset(&info, 0, sizeof(info));
+
+ info.num_of_h_tiles = 1;
+ /* use only WB idx 2 instance for DPU */
+ info.h_tile_instance[0] = WB_2;
+ info.intf_type = INTF_WB;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_VIRTUAL, &info);
if (IS_ERR(encoder)) {
DPU_ERROR("encoder init failed for dsi display\n");
return PTR_ERR(encoder);
}
- memset(&info, 0, sizeof(info));
-
rc = dpu_writeback_init(dev, encoder, wb_formats,
n_formats);
if (rc) {
@@ -680,18 +669,6 @@ static int _dpu_kms_initialize_writeback(struct drm_device *dev,
return rc;
}
- info.num_of_h_tiles = 1;
- /* use only WB idx 2 instance for DPU */
- info.h_tile_instance[0] = WB_2;
- info.intf_type = INTF_WB;
-
- rc = dpu_encoder_setup(dev, encoder, &info);
- if (rc) {
- DPU_ERROR("failed to setup DPU encoder %d: rc:%d\n",
- encoder->base.id, rc);
- return rc;
- }
-
return 0;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/7] drm/msm/dpu: separate common function to init physical encoder
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup() Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-22 22:34 ` Abhinav Kumar
2023-05-19 2:38 ` [PATCH v2 3/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs Dmitry Baryshkov
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
Move common DPU physical encoder initialization code to the new function
dpu_encoder_phys_init().
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +++++++++++++++++--
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 3 ++
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 17 ++---------
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 17 ++---------
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 17 ++---------
5 files changed, 37 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 32785cb1b079..06b34bac4171 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2310,8 +2310,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(&phys->vsync_cnt, 0);
- atomic_set(&phys->underrun_cnt, 0);
if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx);
@@ -2513,3 +2511,30 @@ unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc)
return dpu_enc->dsc_mask;
}
+
+void 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(&phys_enc->vblank_refcount, 0);
+ atomic_set(&phys_enc->pending_kickoff_cnt, 0);
+ atomic_set(&phys_enc->pending_ctlstart_cnt, 0);
+
+ atomic_set(&phys_enc->vsync_cnt, 0);
+ atomic_set(&phys_enc->underrun_cnt, 0);
+
+ init_waitqueue_head(&phys_enc->pending_kickoff_wq);
+}
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..69c3ba5fab26 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);
+void 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..6898664e1381 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,14 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
return ERR_PTR(ret);
}
phys_enc = &cmd_enc->base;
- phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
- phys_enc->intf_idx = p->intf_idx;
+
+ dpu_encoder_phys_init(phys_enc, p);
dpu_encoder_phys_cmd_init_ops(&phys_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(&phys_enc->vblank_refcount, 0);
- atomic_set(&phys_enc->pending_kickoff_cnt, 0);
- atomic_set(&phys_enc->pending_ctlstart_cnt, 0);
atomic_set(&cmd_enc->pending_vblank_cnt, 0);
- init_waitqueue_head(&phys_enc->pending_kickoff_wq);
init_waitqueue_head(&cmd_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..dc951fdf473b 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,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
struct dpu_enc_phys_init_params *p)
{
struct dpu_encoder_phys *phys_enc = NULL;
- int i;
if (!p) {
DPU_ERROR("failed to create encoder due to invalid parameter\n");
@@ -712,24 +711,12 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
return ERR_PTR(-ENOMEM);
}
- phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
- phys_enc->intf_idx = p->intf_idx;
-
DPU_DEBUG_VIDENC(phys_enc, "\n");
+ dpu_encoder_phys_init(phys_enc, p);
+
dpu_encoder_phys_vid_init_ops(&phys_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_VIDEO;
- phys_enc->enc_spinlock = p->enc_spinlock;
- for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
- phys_enc->irq[i] = -EINVAL;
-
- atomic_set(&phys_enc->vblank_refcount, 0);
- atomic_set(&phys_enc->pending_kickoff_cnt, 0);
- init_waitqueue_head(&phys_enc->pending_kickoff_wq);
- phys_enc->enable_state = DPU_ENC_DISABLED;
DPU_DEBUG_VIDENC(phys_enc, "created intf idx:%d\n", p->intf_idx);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 023a9c4ad1db..008d1d09b9ba 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -685,7 +685,6 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_wb *wb_enc = NULL;
int ret = 0;
- int i;
DPU_DEBUG("\n");
@@ -703,28 +702,16 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
}
phys_enc = &wb_enc->base;
- phys_enc->hw_mdptop = p->dpu_kms->hw_mdp;
- phys_enc->wb_idx = p->wb_idx;
+
+ dpu_encoder_phys_init(phys_enc, p);
dpu_encoder_phys_wb_init_ops(&phys_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_WB_LINE;
- phys_enc->wb_idx = p->wb_idx;
- phys_enc->enc_spinlock = p->enc_spinlock;
atomic_set(&wb_enc->wbirq_refcount, 0);
- for (i = 0; i < ARRAY_SIZE(phys_enc->irq); i++)
- phys_enc->irq[i] = -EINVAL;
-
- atomic_set(&phys_enc->pending_kickoff_cnt, 0);
- atomic_set(&phys_enc->vblank_refcount, 0);
wb_enc->wb_done_timeout_cnt = 0;
- init_waitqueue_head(&phys_enc->pending_kickoff_wq);
- phys_enc->enable_state = DPU_ENC_DISABLED;
DPU_DEBUG("Created dpu_encoder_phys for wb %d\n",
phys_enc->wb_idx);
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup() Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 2/7] drm/msm/dpu: separate common function to init physical encoder Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 4/7] drm/msm/dpu: inline dpu_encoder_get_wb() Dmitry Baryshkov
` (4 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
Remove intf_idx and wb_idx fields from struct dpu_encoder_phys and
struct dpu_enc_phys_init_params. Set the hw_intf and hw_wb directly and
use them to get the instance index.
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 72 ++++++++-----------
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 12 ++--
.../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 16 ++---
.../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +-
.../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 8 +--
5 files changed, 46 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 06b34bac4171..267626245a74 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -339,7 +339,8 @@ void dpu_encoder_helper_report_irq_timeout(struct dpu_encoder_phys *phys_enc,
DRM_ERROR("irq timeout id=%u, intf_mode=%s intf=%d wb=%d, pp=%d, intr=%d\n",
DRMID(phys_enc->parent),
dpu_encoder_helper_get_intf_type(phys_enc->intf_mode),
- phys_enc->intf_idx - INTF_0, phys_enc->wb_idx - WB_0,
+ phys_enc->hw_intf ? phys_enc->hw_intf->idx - INTF_0 : -1,
+ phys_enc->hw_wb ? phys_enc->hw_wb->idx - WB_0 : -1,
phys_enc->hw_pp->idx - PINGPONG_0, intr_idx);
dpu_encoder_frame_done_callback(phys_enc->parent, phys_enc,
@@ -1408,7 +1409,8 @@ void dpu_encoder_frame_done_callback(
*/
trace_dpu_enc_frame_done_cb_not_busy(DRMID(drm_enc), event,
dpu_encoder_helper_get_intf_type(ready_phys->intf_mode),
- ready_phys->intf_idx, ready_phys->wb_idx);
+ ready_phys->hw_intf ? ready_phys->hw_intf->idx : -1,
+ ready_phys->hw_wb ? ready_phys->hw_wb->idx : -1);
return;
}
@@ -1488,7 +1490,8 @@ static void _dpu_encoder_trigger_flush(struct drm_encoder *drm_enc,
trace_dpu_enc_trigger_flush(DRMID(drm_enc),
dpu_encoder_helper_get_intf_type(phys->intf_mode),
- phys->intf_idx, phys->wb_idx,
+ phys->hw_intf ? phys->hw_intf->idx : -1,
+ phys->hw_wb ? phys->hw_wb->idx : -1,
pending_kickoff_cnt, ctl->idx,
extra_flush_bits, ret);
}
@@ -2099,7 +2102,8 @@ static int _dpu_encoder_status_show(struct seq_file *s, void *data)
struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
seq_printf(s, "intf:%d wb:%d vsync:%8d underrun:%8d ",
- phys->intf_idx - INTF_0, phys->wb_idx - WB_0,
+ phys->hw_intf ? phys->hw_intf->idx - INTF_0 : -1,
+ phys->hw_wb ? phys->hw_wb->idx - WB_0 : -1,
atomic_read(&phys->vsync_cnt),
atomic_read(&phys->underrun_cnt));
@@ -2263,6 +2267,8 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
* h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right
*/
u32 controller_id = disp_info->h_tile_instance[i];
+ enum dpu_intf intf_idx;
+ enum dpu_wb wb_idx;
if (disp_info->num_of_h_tiles > 1) {
if (i == 0)
@@ -2276,57 +2282,39 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
i, controller_id, phys_params.split_role);
- phys_params.intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
+ intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
disp_info->intf_type,
controller_id);
- phys_params.wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
+ wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
disp_info->intf_type, controller_id);
- /*
- * The phys_params might represent either an INTF or a WB unit, but not
- * both of them at the same time.
- */
- if ((phys_params.intf_idx == INTF_MAX) &&
- (phys_params.wb_idx == WB_MAX)) {
- DPU_ERROR_ENC(dpu_enc, "could not get intf or wb: type %d, id %d\n",
- disp_info->intf_type, controller_id);
- ret = -EINVAL;
- }
- if ((phys_params.intf_idx != INTF_MAX) &&
- (phys_params.wb_idx != WB_MAX)) {
- DPU_ERROR_ENC(dpu_enc, "both intf and wb present: type %d, id %d\n",
- disp_info->intf_type, controller_id);
- ret = -EINVAL;
- }
+ if (intf_idx >= INTF_0 && intf_idx < INTF_MAX)
+ phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx);
- if (!ret) {
- ret = dpu_encoder_virt_add_phys_encs(disp_info,
- dpu_enc, &phys_params);
- if (ret)
- DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n");
- }
- }
+ if (wb_idx >= WB_0 && wb_idx < WB_MAX)
+ phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, wb_idx);
- for (i = 0; i < dpu_enc->num_phys_encs; i++) {
- struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
-
- if (phys->intf_idx >= INTF_0 && phys->intf_idx < INTF_MAX)
- phys->hw_intf = dpu_rm_get_intf(&dpu_kms->rm, phys->intf_idx);
-
- if (phys->wb_idx >= WB_0 && phys->wb_idx < WB_MAX)
- phys->hw_wb = dpu_rm_get_wb(&dpu_kms->rm, phys->wb_idx);
-
- if (!phys->hw_intf && !phys->hw_wb) {
+ if (!phys_params.hw_intf && !phys_params.hw_wb) {
DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i);
ret = -EINVAL;
+ break;
}
- if (phys->hw_intf && phys->hw_wb) {
+ if (phys_params.hw_intf && phys_params.hw_wb) {
DPU_ERROR_ENC(dpu_enc,
"invalid phys both intf and wb block at idx: %d\n", i);
ret = -EINVAL;
+ break;
}
+
+ ret = dpu_encoder_virt_add_phys_encs(disp_info,
+ dpu_enc, &phys_params);
+ if (ret) {
+ DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n");
+ break;
+ }
+
}
mutex_unlock(&dpu_enc->enc_lock);
@@ -2518,8 +2506,8 @@ void dpu_encoder_phys_init(struct dpu_encoder_phys *phys_enc,
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->hw_intf = p->hw_intf;
+ phys_enc->hw_wb = p->hw_wb;
phys_enc->parent = p->parent;
phys_enc->dpu_kms = p->dpu_kms;
phys_enc->split_role = p->split_role;
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 69c3ba5fab26..faa6f69e5ce5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -161,8 +161,6 @@ enum dpu_intr_idx {
* @enabled: Whether the encoder has enabled and running a mode
* @split_role: Role to play in a split-panel configuration
* @intf_mode: Interface mode
- * @intf_idx: Interface index on dpu hardware
- * @wb_idx: Writeback index on dpu hardware
* @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
* @enable_state: Enable state tracking
* @vblank_refcount: Reference count of vblank request
@@ -189,8 +187,6 @@ struct dpu_encoder_phys {
struct drm_display_mode cached_mode;
enum dpu_enc_split_role split_role;
enum dpu_intf_mode intf_mode;
- enum dpu_intf intf_idx;
- enum dpu_wb wb_idx;
spinlock_t *enc_spinlock;
enum dpu_enc_enable_state enable_state;
atomic_t vblank_refcount;
@@ -256,16 +252,16 @@ struct dpu_encoder_phys_cmd {
* @parent: Pointer to the containing virtual encoder
* @parent_ops: Callbacks exposed by the parent to the phys_enc
* @split_role: Role to play in a split-panel configuration
- * @intf_idx: Interface index this phys_enc will control
- * @wb_idx: Writeback index this phys_enc will control
+ * @hw_intf: Hardware interface to the intf registers
+ * @hw_wb: Hardware interface to the wb registers
* @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
*/
struct dpu_enc_phys_init_params {
struct dpu_kms *dpu_kms;
struct drm_encoder *parent;
enum dpu_enc_split_role split_role;
- enum dpu_intf intf_idx;
- enum dpu_wb wb_idx;
+ struct dpu_hw_intf *hw_intf;
+ struct dpu_hw_wb *hw_wb;
spinlock_t *enc_spinlock;
};
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 6898664e1381..bedc8d0316c6 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
@@ -16,12 +16,12 @@
#define DPU_DEBUG_CMDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \
(e) && (e)->base.parent ? \
(e)->base.parent->base.id : -1, \
- (e) ? (e)->base.intf_idx - INTF_0 : -1, ##__VA_ARGS__)
+ (e) ? (e)->base.hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
#define DPU_ERROR_CMDENC(e, fmt, ...) DPU_ERROR("enc%d intf%d " fmt, \
(e) && (e)->base.parent ? \
(e)->base.parent->base.id : -1, \
- (e) ? (e)->base.intf_idx - INTF_0 : -1, ##__VA_ARGS__)
+ (e) ? (e)->base.hw_intf->idx - INTF_0 : -1, ##__VA_ARGS__)
#define to_dpu_encoder_phys_cmd(x) \
container_of(x, struct dpu_encoder_phys_cmd, base)
@@ -59,7 +59,7 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg(
if (!ctl->ops.setup_intf_cfg)
return;
- intf_cfg.intf = phys_enc->intf_idx;
+ intf_cfg.intf = phys_enc->hw_intf->idx;
intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD;
intf_cfg.stream_sel = cmd_enc->stream_sel;
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
@@ -430,7 +430,7 @@ static void dpu_encoder_phys_cmd_enable_helper(
return;
}
- dpu_encoder_helper_split_config(phys_enc, phys_enc->intf_idx);
+ dpu_encoder_helper_split_config(phys_enc, phys_enc->hw_intf->idx);
_dpu_encoder_phys_cmd_pingpong_config(phys_enc);
@@ -438,7 +438,7 @@ static void dpu_encoder_phys_cmd_enable_helper(
return;
ctl = phys_enc->hw_ctl;
- ctl->ops.update_pending_flush_intf(ctl, phys_enc->intf_idx);
+ ctl->ops.update_pending_flush_intf(ctl, phys_enc->hw_intf->idx);
}
static void dpu_encoder_phys_cmd_enable(struct dpu_encoder_phys *phys_enc)
@@ -525,7 +525,7 @@ static void dpu_encoder_phys_cmd_disable(struct dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->idx);
ctl = phys_enc->hw_ctl;
- ctl->ops.update_pending_flush_intf(ctl, phys_enc->intf_idx);
+ ctl->ops.update_pending_flush_intf(ctl, phys_enc->hw_intf->idx);
}
phys_enc->enable_state = DPU_ENC_DISABLED;
@@ -670,7 +670,7 @@ static int dpu_encoder_phys_cmd_wait_for_tx_complete(
if (rc) {
DRM_ERROR("failed wait_for_idle: id:%u ret:%d intf:%d\n",
DRMID(phys_enc->parent), rc,
- phys_enc->intf_idx - INTF_0);
+ phys_enc->hw_intf->idx - INTF_0);
}
return rc;
@@ -761,7 +761,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_cmd_init(
struct dpu_encoder_phys_cmd *cmd_enc = NULL;
int ret = 0;
- DPU_DEBUG("intf %d\n", p->intf_idx - INTF_0);
+ DPU_DEBUG("intf\n");
cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL);
if (!cmd_enc) {
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 dc951fdf473b..e26629e9e303 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
@@ -718,7 +718,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_vid_init(
dpu_encoder_phys_vid_init_ops(&phys_enc->ops);
phys_enc->intf_mode = INTF_MODE_VIDEO;
- DPU_DEBUG_VIDENC(phys_enc, "created intf idx:%d\n", p->intf_idx);
+ DPU_DEBUG_VIDENC(phys_enc, "created intf idx:%d\n", p->hw_intf->idx);
return phys_enc;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 008d1d09b9ba..6608c00e3c33 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -240,7 +240,7 @@ static int dpu_encoder_phys_wb_atomic_check(
const struct drm_display_mode *mode = &crtc_state->mode;
DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n",
- phys_enc->wb_idx, mode->name, mode->hdisplay, mode->vdisplay);
+ phys_enc->hw_wb->idx, mode->name, mode->hdisplay, mode->vdisplay);
if (!conn_state || !conn_state->connector) {
DPU_ERROR("invalid connector state\n");
@@ -561,7 +561,7 @@ static void dpu_encoder_phys_wb_destroy(struct dpu_encoder_phys *phys_enc)
if (!phys_enc)
return;
- DPU_DEBUG("[wb:%d]\n", phys_enc->wb_idx - WB_0);
+ DPU_DEBUG("[wb:%d]\n", phys_enc->hw_wb->idx - WB_0);
kfree(phys_enc);
}
@@ -712,9 +712,7 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
wb_enc->wb_done_timeout_cnt = 0;
-
- DPU_DEBUG("Created dpu_encoder_phys for wb %d\n",
- phys_enc->wb_idx);
+ DPU_DEBUG("Created dpu_encoder_phys for wb %d\n", phys_enc->hw_wb->idx);
return phys_enc;
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 4/7] drm/msm/dpu: inline dpu_encoder_get_wb()
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
` (2 preceding siblings ...)
2023-05-19 2:38 ` [PATCH v2 3/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 5/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf() Dmitry Baryshkov
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
The function dpu_encoder_get_wb() returns controller_id if the
corresponding WB is present in the catalog. We can inline this function
and rely on dpu_rm_get_wb() returning NULL for indices for which the
WB is not present on the device.
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 ++-------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 267626245a74..92e915bcf88a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1277,22 +1277,6 @@ static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog,
return INTF_MAX;
}
-static enum dpu_wb dpu_encoder_get_wb(const struct dpu_mdss_cfg *catalog,
- enum dpu_intf_type type, u32 controller_id)
-{
- int i = 0;
-
- if (type != INTF_WB)
- return WB_MAX;
-
- for (i = 0; i < catalog->wb_count; i++) {
- if (catalog->wb[i].id == controller_id)
- return catalog->wb[i].id;
- }
-
- return WB_MAX;
-}
-
void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
struct dpu_encoder_phys *phy_enc)
{
@@ -2268,7 +2252,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
*/
u32 controller_id = disp_info->h_tile_instance[i];
enum dpu_intf intf_idx;
- enum dpu_wb wb_idx;
if (disp_info->num_of_h_tiles > 1) {
if (i == 0)
@@ -2286,14 +2269,11 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
disp_info->intf_type,
controller_id);
- wb_idx = dpu_encoder_get_wb(dpu_kms->catalog,
- disp_info->intf_type, controller_id);
-
if (intf_idx >= INTF_0 && intf_idx < INTF_MAX)
phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx);
- if (wb_idx >= WB_0 && wb_idx < WB_MAX)
- phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, wb_idx);
+ if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX)
+ phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id);
if (!phys_params.hw_intf && !phys_params.hw_wb) {
DPU_ERROR_ENC(dpu_enc, "no intf or wb block assigned at idx: %d\n", i);
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 5/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
` (3 preceding siblings ...)
2023-05-19 2:38 ` [PATCH v2 4/7] drm/msm/dpu: inline dpu_encoder_get_wb() Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init() Dmitry Baryshkov
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
There is little sense to get intf index just to call dpu_rm_get_intf()
on it. Move dpu_rm_get_intf() call to dpu_encoder_get_intf() function.
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 92e915bcf88a..c72b7445db97 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1259,22 +1259,23 @@ static void dpu_encoder_virt_atomic_disable(struct drm_encoder *drm_enc,
mutex_unlock(&dpu_enc->enc_lock);
}
-static enum dpu_intf dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog,
+static struct dpu_hw_intf *dpu_encoder_get_intf(const struct dpu_mdss_cfg *catalog,
+ struct dpu_rm *dpu_rm,
enum dpu_intf_type type, u32 controller_id)
{
int i = 0;
if (type == INTF_WB)
- return INTF_MAX;
+ return NULL;
for (i = 0; i < catalog->intf_count; i++) {
if (catalog->intf[i].type == type
&& catalog->intf[i].controller_id == controller_id) {
- return catalog->intf[i].id;
+ return dpu_rm_get_intf(dpu_rm, catalog->intf[i].id);
}
}
- return INTF_MAX;
+ return NULL;
}
void dpu_encoder_vblank_callback(struct drm_encoder *drm_enc,
@@ -2251,7 +2252,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
* h_tile_instance_ids[2] = {1, 0}; DSI1 = left, DSI0 = right
*/
u32 controller_id = disp_info->h_tile_instance[i];
- enum dpu_intf intf_idx;
if (disp_info->num_of_h_tiles > 1) {
if (i == 0)
@@ -2265,12 +2265,9 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
DPU_DEBUG("h_tile_instance %d = %d, split_role %d\n",
i, controller_id, phys_params.split_role);
- intf_idx = dpu_encoder_get_intf(dpu_kms->catalog,
- disp_info->intf_type,
- controller_id);
-
- if (intf_idx >= INTF_0 && intf_idx < INTF_MAX)
- phys_params.hw_intf = dpu_rm_get_intf(&dpu_kms->rm, intf_idx);
+ phys_params.hw_intf = dpu_encoder_get_intf(dpu_kms->catalog, &dpu_kms->rm,
+ disp_info->intf_type,
+ controller_id);
if (disp_info->intf_type == INTF_WB && controller_id < WB_MAX)
phys_params.hw_wb = dpu_rm_get_wb(&dpu_kms->rm, controller_id);
@@ -2294,7 +2291,6 @@ static int dpu_encoder_setup_display(struct dpu_encoder_virt *dpu_enc,
DPU_ERROR_ENC(dpu_enc, "failed to add phys encs\n");
break;
}
-
}
mutex_unlock(&dpu_enc->enc_lock);
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
` (4 preceding siblings ...)
2023-05-19 2:38 ` [PATCH v2 5/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf() Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-22 22:38 ` Abhinav Kumar
2023-05-19 2:38 ` [PATCH v2 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init() Dmitry Baryshkov
2023-05-23 1:58 ` [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Abhinav Kumar
7 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
There is no need to assign a result to temp varable just to return it
two lines below. Drop the temporary variable.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
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 bedc8d0316c6..d4685e0a3f8d 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,15 +759,13 @@ 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 ret = 0;
DPU_DEBUG("intf\n");
cmd_enc = kzalloc(sizeof(*cmd_enc), GFP_KERNEL);
if (!cmd_enc) {
- ret = -ENOMEM;
DPU_ERROR("failed to allocate\n");
- return ERR_PTR(ret);
+ return ERR_PTR(-ENOMEM);
}
phys_enc = &cmd_enc->base;
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
` (5 preceding siblings ...)
2023-05-19 2:38 ` [PATCH v2 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init() Dmitry Baryshkov
@ 2023-05-19 2:38 ` Dmitry Baryshkov
2023-05-23 1:56 ` Abhinav Kumar
2023-05-23 1:58 ` [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Abhinav Kumar
7 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-19 2:38 UTC (permalink / raw)
To: Rob Clark, Sean Paul, Abhinav Kumar
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
There is no need to assign a result to temp varable just to return it
after a goto. Drop the temporary variable and goto and return the result
directly.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 6608c00e3c33..e9325cafb1a8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -684,21 +684,18 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
{
struct dpu_encoder_phys *phys_enc = NULL;
struct dpu_encoder_phys_wb *wb_enc = NULL;
- int ret = 0;
DPU_DEBUG("\n");
if (!p || !p->parent) {
DPU_ERROR("invalid params\n");
- ret = -EINVAL;
- goto fail_alloc;
+ return ERR_PTR(-EINVAL);
}
wb_enc = kzalloc(sizeof(*wb_enc), GFP_KERNEL);
if (!wb_enc) {
DPU_ERROR("failed to allocate wb phys_enc enc\n");
- ret = -ENOMEM;
- goto fail_alloc;
+ return ERR_PTR(-ENOMEM);
}
phys_enc = &wb_enc->base;
@@ -715,7 +712,4 @@ struct dpu_encoder_phys *dpu_encoder_phys_wb_init(
DPU_DEBUG("Created dpu_encoder_phys for wb %d\n", phys_enc->hw_wb->idx);
return phys_enc;
-
-fail_alloc:
- return ERR_PTR(ret);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/7] drm/msm/dpu: separate common function to init physical encoder
2023-05-19 2:38 ` [PATCH v2 2/7] drm/msm/dpu: separate common function to init physical encoder Dmitry Baryshkov
@ 2023-05-22 22:34 ` Abhinav Kumar
0 siblings, 0 replies; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-22 22:34 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
> Move common DPU physical encoder initialization code to the new function
> dpu_encoder_phys_init().
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
2023-05-19 2:38 ` [PATCH v2 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init() Dmitry Baryshkov
@ 2023-05-22 22:38 ` Abhinav Kumar
0 siblings, 0 replies; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-22 22:38 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
> There is no need to assign a result to temp varable just to return it
> two lines below. Drop the temporary variable.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
2023-05-19 2:38 ` [PATCH v2 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init() Dmitry Baryshkov
@ 2023-05-23 1:56 ` Abhinav Kumar
0 siblings, 0 replies; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-23 1:56 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul
Cc: freedreno, linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd
On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
> There is no need to assign a result to temp varable just to return it
> after a goto. Drop the temporary variable and goto and return the result
> directly.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
` (6 preceding siblings ...)
2023-05-19 2:38 ` [PATCH v2 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init() Dmitry Baryshkov
@ 2023-05-23 1:58 ` Abhinav Kumar
2023-05-23 7:20 ` Dmitry Baryshkov
7 siblings, 1 reply; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-23 1:58 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Sean Paul
Cc: linux-arm-msm, Bjorn Andersson, dri-devel, Stephen Boyd, freedreno
On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
> Rework dpu_encoder initialization code, simplifying calling sequences
> and separating common init parts.
>
> Changes since v1:
> - Withdrawn two pathes for a later consideration
> - Changed dpu_encoder_phys_init() to return void (Abhinav)
> - Added small simplifications of dpu_encoder_phys_cmd_init() and
> dpu_encoder_phys_wb_init()
>
I had previously given these comments on the cover letter of v1, so
giving it again.
Please mention that your series was made on top of
https://patchwork.freedesktop.org/series/116530/.
Figured it out when I tried to apply it to my branch to test.
I had tested v1, and between v1 and v2 i only see very trivial change,
so i think its okay to retain:
Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
> Dmitry Baryshkov (7):
> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
> drm/msm/dpu: separate common function to init physical encoder
> drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
> drm/msm/dpu: inline dpu_encoder_get_wb()
> drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
> drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178 ++++++++----------
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
> 7 files changed, 140 insertions(+), 243 deletions(-)
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 1:58 ` [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Abhinav Kumar
@ 2023-05-23 7:20 ` Dmitry Baryshkov
2023-05-23 7:31 ` Neil Armstrong
0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-23 7:20 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, freedreno
On Tue, 23 May 2023 at 04:58, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
> > Rework dpu_encoder initialization code, simplifying calling sequences
> > and separating common init parts.
> >
> > Changes since v1:
> > - Withdrawn two pathes for a later consideration
> > - Changed dpu_encoder_phys_init() to return void (Abhinav)
> > - Added small simplifications of dpu_encoder_phys_cmd_init() and
> > dpu_encoder_phys_wb_init()
> >
>
> I had previously given these comments on the cover letter of v1, so
> giving it again.
>
> Please mention that your series was made on top of
> https://patchwork.freedesktop.org/series/116530/.
>
> Figured it out when I tried to apply it to my branch to test.
>
> I had tested v1, and between v1 and v2 i only see very trivial change,
> so i think its okay to retain:
>
> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
Unfortunately patchwork ignores tags sent in the cover letter thread.
>
> > Dmitry Baryshkov (7):
> > drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
> > drm/msm/dpu: separate common function to init physical encoder
> > drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
> > drm/msm/dpu: inline dpu_encoder_get_wb()
> > drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
> > drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
> > drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
> >
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178 ++++++++----------
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
> > .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
> > .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
> > .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
> > 7 files changed, 140 insertions(+), 243 deletions(-)
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 7:20 ` Dmitry Baryshkov
@ 2023-05-23 7:31 ` Neil Armstrong
2023-05-23 14:36 ` Dmitry Baryshkov
0 siblings, 1 reply; 19+ messages in thread
From: Neil Armstrong @ 2023-05-23 7:31 UTC (permalink / raw)
To: Dmitry Baryshkov, Abhinav Kumar
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, freedreno
On 23/05/2023 09:20, Dmitry Baryshkov wrote:
> On Tue, 23 May 2023 at 04:58, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>>
>>
>> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
>>> Rework dpu_encoder initialization code, simplifying calling sequences
>>> and separating common init parts.
>>>
>>> Changes since v1:
>>> - Withdrawn two pathes for a later consideration
>>> - Changed dpu_encoder_phys_init() to return void (Abhinav)
>>> - Added small simplifications of dpu_encoder_phys_cmd_init() and
>>> dpu_encoder_phys_wb_init()
>>>
>>
>> I had previously given these comments on the cover letter of v1, so
>> giving it again.
>>
>> Please mention that your series was made on top of
>> https://patchwork.freedesktop.org/series/116530/.
>>
>> Figured it out when I tried to apply it to my branch to test.
>>
>> I had tested v1, and between v1 and v2 i only see very trivial change,
>> so i think its okay to retain:
>>
>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>
> Unfortunately patchwork ignores tags sent in the cover letter thread.
But b4 does with -t option to b4 shazam or b4 am
Neil
>
>>
>>> Dmitry Baryshkov (7):
>>> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
>>> drm/msm/dpu: separate common function to init physical encoder
>>> drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
>>> drm/msm/dpu: inline dpu_encoder_get_wb()
>>> drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
>>> drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
>>> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
>>>
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178 ++++++++----------
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
>>> 7 files changed, 140 insertions(+), 243 deletions(-)
>>>
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 7:31 ` Neil Armstrong
@ 2023-05-23 14:36 ` Dmitry Baryshkov
2023-05-23 19:13 ` Abhinav Kumar
0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-23 14:36 UTC (permalink / raw)
To: neil.armstrong, Abhinav Kumar
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, freedreno
On 23/05/2023 10:31, Neil Armstrong wrote:
> On 23/05/2023 09:20, Dmitry Baryshkov wrote:
>> On Tue, 23 May 2023 at 04:58, Abhinav Kumar
>> <quic_abhinavk@quicinc.com> wrote:
>>>
>>>
>>>
>>> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
>>>> Rework dpu_encoder initialization code, simplifying calling sequences
>>>> and separating common init parts.
>>>>
>>>> Changes since v1:
>>>> - Withdrawn two pathes for a later consideration
>>>> - Changed dpu_encoder_phys_init() to return void (Abhinav)
>>>> - Added small simplifications of dpu_encoder_phys_cmd_init() and
>>>> dpu_encoder_phys_wb_init()
>>>>
>>>
>>> I had previously given these comments on the cover letter of v1, so
>>> giving it again.
>>>
>>> Please mention that your series was made on top of
>>> https://patchwork.freedesktop.org/series/116530/.
>>>
>>> Figured it out when I tried to apply it to my branch to test.
>>>
>>> I had tested v1, and between v1 and v2 i only see very trivial change,
>>> so i think its okay to retain:
>>>
>>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>>
>> Unfortunately patchwork ignores tags sent in the cover letter thread.
>
> But b4 does with -t option to b4 shazam or b4 am
Yes. But b4 doesn't append Patchwork headers.
>
> Neil
>
>>
>>>
>>>> Dmitry Baryshkov (7):
>>>> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
>>>> drm/msm/dpu: separate common function to init physical encoder
>>>> drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
>>>> drm/msm/dpu: inline dpu_encoder_get_wb()
>>>> drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
>>>> drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
>>>> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
>>>>
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178
>>>> ++++++++----------
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
>>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
>>>> 7 files changed, 140 insertions(+), 243 deletions(-)
>>>>
>>
>>
>>
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 14:36 ` Dmitry Baryshkov
@ 2023-05-23 19:13 ` Abhinav Kumar
2023-05-23 19:23 ` Dmitry Baryshkov
0 siblings, 1 reply; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-23 19:13 UTC (permalink / raw)
To: Dmitry Baryshkov, neil.armstrong
Cc: Sean Paul, Bjorn Andersson, dri-devel, Stephen Boyd,
linux-arm-msm, freedreno
On 5/23/2023 7:36 AM, Dmitry Baryshkov wrote:
> On 23/05/2023 10:31, Neil Armstrong wrote:
>> On 23/05/2023 09:20, Dmitry Baryshkov wrote:
>>> On Tue, 23 May 2023 at 04:58, Abhinav Kumar
>>> <quic_abhinavk@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
>>>>> Rework dpu_encoder initialization code, simplifying calling sequences
>>>>> and separating common init parts.
>>>>>
>>>>> Changes since v1:
>>>>> - Withdrawn two pathes for a later consideration
>>>>> - Changed dpu_encoder_phys_init() to return void (Abhinav)
>>>>> - Added small simplifications of dpu_encoder_phys_cmd_init() and
>>>>> dpu_encoder_phys_wb_init()
>>>>>
>>>>
>>>> I had previously given these comments on the cover letter of v1, so
>>>> giving it again.
>>>>
>>>> Please mention that your series was made on top of
>>>> https://patchwork.freedesktop.org/series/116530/.
>>>>
>>>> Figured it out when I tried to apply it to my branch to test.
>>>>
>>>> I had tested v1, and between v1 and v2 i only see very trivial change,
>>>> so i think its okay to retain:
>>>>
>>>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>>>
>>> Unfortunately patchwork ignores tags sent in the cover letter thread.
>>
>> But b4 does with -t option to b4 shazam or b4 am
>
> Yes. But b4 doesn't append Patchwork headers.
>
If thats the case, either the author can add them to the patches
manually like we do sometimes for R-b tags OR I will go ahead and add it
one by one for every patch now.
Let me know what you prefer.
>>
>> Neil
>>
>>>
>>>>
>>>>> Dmitry Baryshkov (7):
>>>>> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
>>>>> drm/msm/dpu: separate common function to init physical encoder
>>>>> drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
>>>>> drm/msm/dpu: inline dpu_encoder_get_wb()
>>>>> drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
>>>>> drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
>>>>> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
>>>>>
>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178
>>>>> ++++++++----------
>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
>>>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
>>>>> 7 files changed, 140 insertions(+), 243 deletions(-)
>>>>>
>>>
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 19:13 ` Abhinav Kumar
@ 2023-05-23 19:23 ` Dmitry Baryshkov
2023-05-23 19:30 ` Abhinav Kumar
0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Baryshkov @ 2023-05-23 19:23 UTC (permalink / raw)
To: Abhinav Kumar
Cc: neil.armstrong, Sean Paul, Bjorn Andersson, dri-devel,
Stephen Boyd, linux-arm-msm, freedreno
On Tue, 23 May 2023 at 22:14, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
>
>
> On 5/23/2023 7:36 AM, Dmitry Baryshkov wrote:
> > On 23/05/2023 10:31, Neil Armstrong wrote:
> >> On 23/05/2023 09:20, Dmitry Baryshkov wrote:
> >>> On Tue, 23 May 2023 at 04:58, Abhinav Kumar
> >>> <quic_abhinavk@quicinc.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
> >>>>> Rework dpu_encoder initialization code, simplifying calling sequences
> >>>>> and separating common init parts.
> >>>>>
> >>>>> Changes since v1:
> >>>>> - Withdrawn two pathes for a later consideration
> >>>>> - Changed dpu_encoder_phys_init() to return void (Abhinav)
> >>>>> - Added small simplifications of dpu_encoder_phys_cmd_init() and
> >>>>> dpu_encoder_phys_wb_init()
> >>>>>
> >>>>
> >>>> I had previously given these comments on the cover letter of v1, so
> >>>> giving it again.
> >>>>
> >>>> Please mention that your series was made on top of
> >>>> https://patchwork.freedesktop.org/series/116530/.
> >>>>
> >>>> Figured it out when I tried to apply it to my branch to test.
> >>>>
> >>>> I had tested v1, and between v1 and v2 i only see very trivial change,
> >>>> so i think its okay to retain:
> >>>>
> >>>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
> >>>
> >>> Unfortunately patchwork ignores tags sent in the cover letter thread.
> >>
> >> But b4 does with -t option to b4 shazam or b4 am
> >
> > Yes. But b4 doesn't append Patchwork headers.
> >
>
> If thats the case, either the author can add them to the patches
> manually like we do sometimes for R-b tags OR I will go ahead and add it
> one by one for every patch now.
I'd prefer either to have a single T-B on the latest patch on the
series, or a pile of replies with T-B tags. Thank you (for the testing
and for providing the feedback).
If we ever switch from git-pw to b4, this requirement will be lifted.
>
> Let me know what you prefer.
>
> >>
> >> Neil
> >>
> >>>
> >>>>
> >>>>> Dmitry Baryshkov (7):
> >>>>> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
> >>>>> drm/msm/dpu: separate common function to init physical encoder
> >>>>> drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
> >>>>> drm/msm/dpu: inline dpu_encoder_get_wb()
> >>>>> drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
> >>>>> drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
> >>>>> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
> >>>>>
> >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178
> >>>>> ++++++++----------
> >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
> >>>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
> >>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
> >>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
> >>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
> >>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
> >>>>> 7 files changed, 140 insertions(+), 243 deletions(-)
> >>>>>
> >>>
> >>>
> >>>
> >>
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 19:23 ` Dmitry Baryshkov
@ 2023-05-23 19:30 ` Abhinav Kumar
2023-05-30 23:51 ` Abhinav Kumar
0 siblings, 1 reply; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-23 19:30 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: neil.armstrong, linux-arm-msm, Bjorn Andersson, dri-devel,
Stephen Boyd, freedreno, Sean Paul
On 5/23/2023 12:23 PM, Dmitry Baryshkov wrote:
> On Tue, 23 May 2023 at 22:14, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>>
>>
>>
>> On 5/23/2023 7:36 AM, Dmitry Baryshkov wrote:
>>> On 23/05/2023 10:31, Neil Armstrong wrote:
>>>> On 23/05/2023 09:20, Dmitry Baryshkov wrote:
>>>>> On Tue, 23 May 2023 at 04:58, Abhinav Kumar
>>>>> <quic_abhinavk@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
>>>>>>> Rework dpu_encoder initialization code, simplifying calling sequences
>>>>>>> and separating common init parts.
>>>>>>>
>>>>>>> Changes since v1:
>>>>>>> - Withdrawn two pathes for a later consideration
>>>>>>> - Changed dpu_encoder_phys_init() to return void (Abhinav)
>>>>>>> - Added small simplifications of dpu_encoder_phys_cmd_init() and
>>>>>>> dpu_encoder_phys_wb_init()
>>>>>>>
>>>>>>
>>>>>> I had previously given these comments on the cover letter of v1, so
>>>>>> giving it again.
>>>>>>
>>>>>> Please mention that your series was made on top of
>>>>>> https://patchwork.freedesktop.org/series/116530/.
>>>>>>
>>>>>> Figured it out when I tried to apply it to my branch to test.
>>>>>>
>>>>>> I had tested v1, and between v1 and v2 i only see very trivial change,
>>>>>> so i think its okay to retain:
>>>>>>
>>>>>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>>>>>
>>>>> Unfortunately patchwork ignores tags sent in the cover letter thread.
>>>>
>>>> But b4 does with -t option to b4 shazam or b4 am
>>>
>>> Yes. But b4 doesn't append Patchwork headers.
>>>
>>
>> If thats the case, either the author can add them to the patches
>> manually like we do sometimes for R-b tags OR I will go ahead and add it
>> one by one for every patch now.
>
> I'd prefer either to have a single T-B on the latest patch on the
> series, or a pile of replies with T-B tags. Thank you (for the testing
> and for providing the feedback).
> If we ever switch from git-pw to b4, this requirement will be lifted.
>
Latest patch means, the last one in the series?
In this case, that would look a bit odd as that one just removes a temp
variable.
I will provide it on all the patches by tomorrow.
>>
>> Let me know what you prefer.
>>
>>>>
>>>> Neil
>>>>
>>>>>
>>>>>>
>>>>>>> Dmitry Baryshkov (7):
>>>>>>> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
>>>>>>> drm/msm/dpu: separate common function to init physical encoder
>>>>>>> drm/msm/dpu: drop duplicated intf/wb indices from encoder structs
>>>>>>> drm/msm/dpu: inline dpu_encoder_get_wb()
>>>>>>> drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf()
>>>>>>> drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init()
>>>>>>> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
>>>>>>>
>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178
>>>>>>> ++++++++----------
>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
>>>>>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
>>>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
>>>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
>>>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
>>>>>>> 7 files changed, 140 insertions(+), 243 deletions(-)
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init
2023-05-23 19:30 ` Abhinav Kumar
@ 2023-05-30 23:51 ` Abhinav Kumar
0 siblings, 0 replies; 19+ messages in thread
From: Abhinav Kumar @ 2023-05-30 23:51 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Sean Paul, neil.armstrong, linux-arm-msm, Bjorn Andersson,
dri-devel, Stephen Boyd, freedreno
On 5/23/2023 12:30 PM, Abhinav Kumar wrote:
>
>
> On 5/23/2023 12:23 PM, Dmitry Baryshkov wrote:
>> On Tue, 23 May 2023 at 22:14, Abhinav Kumar
>> <quic_abhinavk@quicinc.com> wrote:
>>>
>>>
>>>
>>> On 5/23/2023 7:36 AM, Dmitry Baryshkov wrote:
>>>> On 23/05/2023 10:31, Neil Armstrong wrote:
>>>>> On 23/05/2023 09:20, Dmitry Baryshkov wrote:
>>>>>> On Tue, 23 May 2023 at 04:58, Abhinav Kumar
>>>>>> <quic_abhinavk@quicinc.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 5/18/2023 7:38 PM, Dmitry Baryshkov wrote:
>>>>>>>> Rework dpu_encoder initialization code, simplifying calling
>>>>>>>> sequences
>>>>>>>> and separating common init parts.
>>>>>>>>
>>>>>>>> Changes since v1:
>>>>>>>> - Withdrawn two pathes for a later consideration
>>>>>>>> - Changed dpu_encoder_phys_init() to return void (Abhinav)
>>>>>>>> - Added small simplifications of dpu_encoder_phys_cmd_init() and
>>>>>>>> dpu_encoder_phys_wb_init()
>>>>>>>>
>>>>>>>
>>>>>>> I had previously given these comments on the cover letter of v1, so
>>>>>>> giving it again.
>>>>>>>
>>>>>>> Please mention that your series was made on top of
>>>>>>> https://patchwork.freedesktop.org/series/116530/.
>>>>>>>
>>>>>>> Figured it out when I tried to apply it to my branch to test.
>>>>>>>
>>>>>>> I had tested v1, and between v1 and v2 i only see very trivial
>>>>>>> change,
>>>>>>> so i think its okay to retain:
>>>>>>>
>>>>>>> Tested-by: Abhinav Kumar <quic_abhinavk@quicinc.com> # sc7280
>>>>>>
>>>>>> Unfortunately patchwork ignores tags sent in the cover letter thread.
>>>>>
>>>>> But b4 does with -t option to b4 shazam or b4 am
>>>>
>>>> Yes. But b4 doesn't append Patchwork headers.
>>>>
>>>
>>> If thats the case, either the author can add them to the patches
>>> manually like we do sometimes for R-b tags OR I will go ahead and add it
>>> one by one for every patch now.
>>
>> I'd prefer either to have a single T-B on the latest patch on the
>> series, or a pile of replies with T-B tags. Thank you (for the testing
>> and for providing the feedback).
>> If we ever switch from git-pw to b4, this requirement will be lifted.
>>
>
> Latest patch means, the last one in the series?
>
> In this case, that would look a bit odd as that one just removes a temp
> variable.
>
> I will provide it on all the patches by tomorrow.
>
This patch doesnt apply cleanly on msm-next-lumag now. Do you have to
rebase this? Otherwise please list the dependency on top of msm-next-lumag.
>>>
>>> Let me know what you prefer.
>>>
>>>>>
>>>>> Neil
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> Dmitry Baryshkov (7):
>>>>>>>> drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()
>>>>>>>> drm/msm/dpu: separate common function to init physical encoder
>>>>>>>> drm/msm/dpu: drop duplicated intf/wb indices from encoder
>>>>>>>> structs
>>>>>>>> drm/msm/dpu: inline dpu_encoder_get_wb()
>>>>>>>> drm/msm/dpu: call dpu_rm_get_intf() from
>>>>>>>> dpu_encoder_get_intf()
>>>>>>>> drm/msm/dpu: drop temp variable from
>>>>>>>> dpu_encoder_phys_cmd_init()
>>>>>>>> drm/msm/dpu: simplify dpu_encoder_phys_wb_init()
>>>>>>>>
>>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 178
>>>>>>>> ++++++++----------
>>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 14 +-
>>>>>>>> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 15 +-
>>>>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 35 ++--
>>>>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 19 +-
>>>>>>>> .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 35 +---
>>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 87 ++++-----
>>>>>>>> 7 files changed, 140 insertions(+), 243 deletions(-)
>>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-05-30 23:52 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-19 2:38 [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup() Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 2/7] drm/msm/dpu: separate common function to init physical encoder Dmitry Baryshkov
2023-05-22 22:34 ` Abhinav Kumar
2023-05-19 2:38 ` [PATCH v2 3/7] drm/msm/dpu: drop duplicated intf/wb indices from encoder structs Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 4/7] drm/msm/dpu: inline dpu_encoder_get_wb() Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 5/7] drm/msm/dpu: call dpu_rm_get_intf() from dpu_encoder_get_intf() Dmitry Baryshkov
2023-05-19 2:38 ` [PATCH v2 6/7] drm/msm/dpu: drop temp variable from dpu_encoder_phys_cmd_init() Dmitry Baryshkov
2023-05-22 22:38 ` Abhinav Kumar
2023-05-19 2:38 ` [PATCH v2 7/7] drm/msm/dpu: simplify dpu_encoder_phys_wb_init() Dmitry Baryshkov
2023-05-23 1:56 ` Abhinav Kumar
2023-05-23 1:58 ` [Freedreno] [PATCH v2 0/7] drm/msm/dpu: simplify DPU encoder init Abhinav Kumar
2023-05-23 7:20 ` Dmitry Baryshkov
2023-05-23 7:31 ` Neil Armstrong
2023-05-23 14:36 ` Dmitry Baryshkov
2023-05-23 19:13 ` Abhinav Kumar
2023-05-23 19:23 ` Dmitry Baryshkov
2023-05-23 19:30 ` Abhinav Kumar
2023-05-30 23:51 ` Abhinav Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).