Messages in this thread Patch in this message |  | | Date | Tue, 26 May 2026 12:46:30 -0600 | | Subject | Re: [PATCH v5 2/8] drm/amd/display: use drmm_writeback_connector_init() | | From | Alex Hung <> |
| |
Will allocating wbcon with drmm_kzalloc before calling amdgpu_dm_wb_connector_init be more memory-safe as below?
@@ -5790,7 +5791,8 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev) link = dc_get_link_at_index(dm->dc, i);
if (link->connector_signal == SIGNAL_TYPE_VIRTUAL) { - struct amdgpu_dm_wb_connector *wbcon = kzalloc_obj(*wbcon); + struct amdgpu_dm_wb_connector *wbcon = + drmm_kzalloc(adev_to_drm(adev), sizeof(*wbcon), GFP_KERNEL);
if (!wbcon) { drm_err(adev_to_drm(adev), "KMS: Failed to allocate writeback connector\n"); @@ -5799,7 +5801,6 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
if (amdgpu_dm_wb_connector_init(dm, wbcon, i)) { drm_err(adev_to_drm(adev), "KMS: Failed to initialize writeback connector\n"); - kfree(wbcon); continue; }
On 5/4/26 18:24, Dmitry Baryshkov wrote: > The driver uses drm_writeback_connector_init() instead of its drmm > counterpart, but it doesn't perform the job queue cleanup (neither > manually nor by calling drm_writeback_connector_cleanup()). On the > contrary, the drmm_writeback_connector_init() function ensures the > proper cleanup of the job queue. > Use drmm_plain_encoder_alloc() to allocate simple encoder and > drmm_writeback_connector_init() in order to initialize writeback > connector instance. > > Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com> > Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c | 18 +++++++++++++----- > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index e96a12ff2d31..2ac64495cdb7 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -10683,7 +10683,7 @@ static void dm_set_writeback(struct amdgpu_display_manager *dm, > return; > } > > - acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc); > + acrtc = to_amdgpu_crtc(crtc_state->base.crtc); > if (!acrtc) { > drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n"); > kfree(wb_info); > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > index 110f0173eee6..fdc3da40452f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c > @@ -169,7 +169,6 @@ static const struct drm_encoder_helper_funcs amdgpu_dm_wb_encoder_helper_funcs = > > static const struct drm_connector_funcs amdgpu_dm_wb_connector_funcs = { > .fill_modes = drm_helper_probe_single_connector_modes, > - .destroy = drm_connector_cleanup, > .reset = amdgpu_dm_connector_funcs_reset, > .atomic_duplicate_state = amdgpu_dm_connector_atomic_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > @@ -188,17 +187,26 @@ int amdgpu_dm_wb_connector_init(struct amdgpu_display_manager *dm, > struct dc *dc = dm->dc; > struct dc_link *link = dc_get_link_at_index(dc, link_index); > int res = 0; > + struct drm_encoder *encoder; > + > + encoder = drmm_plain_encoder_alloc(&dm->adev->ddev, NULL, > + DRM_MODE_ENCODER_VIRTUAL, NULL); > + if (IS_ERR(encoder)) > + return PTR_ERR(encoder); > + > + drm_encoder_helper_add(encoder, &amdgpu_dm_wb_encoder_helper_funcs); > + > + encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm->adev); > > wbcon->link = link; > > drm_connector_helper_add(&wbcon->base.base, &amdgpu_dm_wb_conn_helper_funcs); > > - res = drm_writeback_connector_init(&dm->adev->ddev, &wbcon->base, > + res = drmm_writeback_connector_init(&dm->adev->ddev, &wbcon->base, > &amdgpu_dm_wb_connector_funcs, > - &amdgpu_dm_wb_encoder_helper_funcs, > + encoder, > amdgpu_dm_wb_formats, > - ARRAY_SIZE(amdgpu_dm_wb_formats), > - amdgpu_dm_get_encoder_crtc_mask(dm->adev)); > + ARRAY_SIZE(amdgpu_dm_wb_formats)); > > if (res) > return res; >
|  |