Re: [PATCH v3 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()

2023-06-01 Thread Abhinav Kumar




On 6/1/2023 10:22 AM, Dmitry Baryshkov wrote:

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 
Signed-off-by: Dmitry Baryshkov 
---



Tested-by: Abhinav Kumar  # sc7280


[PATCH v3 1/7] drm/msm/dpu: merge dpu_encoder_init() and dpu_encoder_setup()

2023-06-01 Thread Dmitry Baryshkov
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 
Signed-off-by: Dmitry Baryshkov 
---
 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 d7cd4734dc7d..d4b759e8f2ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2388,7 +2388,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;
@@ -2397,7 +2398,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)
@@ -2426,44 +2443,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 8ce057cc9374..70a17ef8281f 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);
+