| Message ID | 20230730011920.354575-14-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm/dpu: use managed memory allocations | expand |
On 7/29/2023 6:19 PM, Dmitry Baryshkov wrote: > It is incorrect to use devm-managed memory allocations for DRM data > structures exposed to userspace. They should use drmm_ allocations. > Change struct dpu_encoder allocation to use drmm_encoder_alloc(). This > removes the need to perform any actions on encoder destruction. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 50 +++++---------------- > 1 file changed, 10 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 2b94ff3d08a1..6d9ec3ac065c 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -443,23 +443,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc) > return linecount; > } > > -static void dpu_encoder_destroy(struct drm_encoder *drm_enc) > -{ > - struct dpu_encoder_virt *dpu_enc = NULL; > - int i = 0; > - > - if (!drm_enc) { > - DPU_ERROR("invalid encoder\n"); > - return; > - } > - > - dpu_enc = to_dpu_encoder_virt(drm_enc); > - DPU_DEBUG_ENC(dpu_enc, "\n"); > - > - drm_encoder_cleanup(drm_enc); > - mutex_destroy(&dpu_enc->enc_lock); > -} > - > void dpu_encoder_helper_split_config( > struct dpu_encoder_phys *phys_enc, > enum dpu_intf interface) > @@ -2381,7 +2364,6 @@ static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = { > }; > > static const struct drm_encoder_funcs dpu_encoder_funcs = { > - .destroy = dpu_encoder_destroy, > .late_register = dpu_encoder_late_register, > .early_unregister = dpu_encoder_early_unregister, > }; > @@ -2392,20 +2374,13 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > { > struct msm_drm_private *priv = dev->dev_private; > struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > - struct drm_encoder *drm_enc = NULL; > - struct dpu_encoder_virt *dpu_enc = NULL; > - int ret = 0; > + struct dpu_encoder_virt *dpu_enc; > + int ret; > > - 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); > - } > + dpu_enc = drmm_encoder_alloc(dev, struct dpu_encoder_virt, base, > + &dpu_encoder_funcs, drm_enc_mode, NULL); > + if (IS_ERR(dpu_enc)) > + return ERR_CAST(dpu_enc); > > drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs); > > @@ -2415,8 +2390,10 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > mutex_init(&dpu_enc->rc_lock); > > ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info); > - if (ret) > - goto fail; > + if (ret) { > + DPU_ERROR("failed to setup encoder\n"); > + return ERR_PTR(-ENOMEM); > + } > > atomic_set(&dpu_enc->frame_done_timeout_ms, 0); > timer_setup(&dpu_enc->frame_done_timer, > @@ -2442,13 +2419,6 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, > DPU_DEBUG_ENC(dpu_enc, "created\n"); > > return &dpu_enc->base; > - > -fail: > - DPU_ERROR("failed to create encoder\n"); > - if (drm_enc) > - dpu_encoder_destroy(drm_enc); > - > - return ERR_PTR(ret); > } > > int dpu_encoder_wait_for_event(struct drm_encoder *drm_enc, > -- > 2.39.2 >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 2b94ff3d08a1..6d9ec3ac065c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -443,23 +443,6 @@ int dpu_encoder_get_linecount(struct drm_encoder *drm_enc) return linecount; } -static void dpu_encoder_destroy(struct drm_encoder *drm_enc) -{ - struct dpu_encoder_virt *dpu_enc = NULL; - int i = 0; - - if (!drm_enc) { - DPU_ERROR("invalid encoder\n"); - return; - } - - dpu_enc = to_dpu_encoder_virt(drm_enc); - DPU_DEBUG_ENC(dpu_enc, "\n"); - - drm_encoder_cleanup(drm_enc); - mutex_destroy(&dpu_enc->enc_lock); -} - void dpu_encoder_helper_split_config( struct dpu_encoder_phys *phys_enc, enum dpu_intf interface) @@ -2381,7 +2364,6 @@ static const struct drm_encoder_helper_funcs dpu_encoder_helper_funcs = { }; static const struct drm_encoder_funcs dpu_encoder_funcs = { - .destroy = dpu_encoder_destroy, .late_register = dpu_encoder_late_register, .early_unregister = dpu_encoder_early_unregister, }; @@ -2392,20 +2374,13 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, { struct msm_drm_private *priv = dev->dev_private; struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); - struct drm_encoder *drm_enc = NULL; - struct dpu_encoder_virt *dpu_enc = NULL; - int ret = 0; + struct dpu_encoder_virt *dpu_enc; + int ret; - 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); - } + dpu_enc = drmm_encoder_alloc(dev, struct dpu_encoder_virt, base, + &dpu_encoder_funcs, drm_enc_mode, NULL); + if (IS_ERR(dpu_enc)) + return ERR_CAST(dpu_enc); drm_encoder_helper_add(&dpu_enc->base, &dpu_encoder_helper_funcs); @@ -2415,8 +2390,10 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, mutex_init(&dpu_enc->rc_lock); ret = dpu_encoder_setup_display(dpu_enc, dpu_kms, disp_info); - if (ret) - goto fail; + if (ret) { + DPU_ERROR("failed to setup encoder\n"); + return ERR_PTR(-ENOMEM); + } atomic_set(&dpu_enc->frame_done_timeout_ms, 0); timer_setup(&dpu_enc->frame_done_timer, @@ -2442,13 +2419,6 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, DPU_DEBUG_ENC(dpu_enc, "created\n"); return &dpu_enc->base; - -fail: - DPU_ERROR("failed to create encoder\n"); - if (drm_enc) - dpu_encoder_destroy(drm_enc); - - return ERR_PTR(ret); } int dpu_encoder_wait_for_event(struct drm_encoder *drm_enc,
It is incorrect to use devm-managed memory allocations for DRM data structures exposed to userspace. They should use drmm_ allocations. Change struct dpu_encoder allocation to use drmm_encoder_alloc(). This removes the need to perform any actions on encoder destruction. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 50 +++++---------------- 1 file changed, 10 insertions(+), 40 deletions(-)