Re: [PATCH v7 1/9] drm/amd/display: embed the writeback encoder in amdgpu_dm_wb_connector

[Harry Wentland <harry.wentland@amd.com>]

On 10/19/26 09:12, Dmitry Baryshkov wrote:
> drm_writeback_connector_init() hides the encoder inside struct
> drm_writeback_connector, which is the only reason that structure carries
> an encoder at all. Every other writeback user already owns its encoder.
>
> Embed the encoder in the driver's own structure and register the
> connector with drm_writeback_connector_init_with_encoder(), so that the
> encoder can be dropped from drm_writeback_connector.
>
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h  |  1 +
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_wb.c   | 29 ++++++++++++++++------
>  .../amd/display/amdgpu_dm/tests/amdgpu_dm_test.c   |  4 +--
>  .../display/amdgpu_dm/tests/amdgpu_dm_wb_test.c    |  4 +--
>  5 files changed, 27 insertions(+), 13 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 5e106b65034d..07dd69b42e83 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4805,7 +4805,7 @@ dm_set_writeback(struct amdgpu_display_manager *dm,
>  		goto cleanup;
>  	}
>  
> -	acrtc = to_amdgpu_crtc(wb_conn->encoder.crtc);
> +	acrtc = to_amdgpu_crtc(to_amdgpu_dm_wb_connector(wb_conn)->encoder.crtc);
>  	if (!acrtc) {
>  		drm_err(adev_to_drm(adev), "no amdgpu_crtc found\n");
>  		goto cleanup;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 1d256dce0e4b..d365370d9958 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -930,6 +930,7 @@ static inline void amdgpu_dm_set_mst_status(uint8_t *status,
>  
>  struct amdgpu_dm_wb_connector {
>  	struct drm_writeback_connector base;
> +	struct drm_encoder encoder;
>  	struct dc_link *link;
>  };
>  
> 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 6f91a992d43f..b5b186e4f81c 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
> @@ -218,6 +218,10 @@ STATIC_IFN_KUNIT void amdgpu_dm_wb_cleanup_job(struct drm_writeback_connector *c
>  }
>  EXPORT_IF_KUNIT(amdgpu_dm_wb_cleanup_job);
>  
> +static const struct drm_encoder_funcs amdgpu_dm_wb_encoder_funcs = {
> +	.destroy = drm_encoder_cleanup,
> +};
> +
>  static const struct drm_encoder_helper_funcs amdgpu_dm_wb_encoder_helper_funcs = {
>  	.atomic_check = amdgpu_dm_wb_encoder_atomic_check,
>  };
> @@ -246,17 +250,26 @@ int amdgpu_dm_wb_connector_init(struct amdgpu_display_manager *dm,
>  
>  	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,
> -					    &amdgpu_dm_wb_connector_funcs,
> -					    &amdgpu_dm_wb_encoder_helper_funcs,
> -					    amdgpu_dm_wb_formats,
> -					    ARRAY_SIZE(amdgpu_dm_wb_formats),
> -					    amdgpu_dm_get_encoder_crtc_mask(dm->adev));
> +	drm_encoder_helper_add(&wbcon->encoder, &amdgpu_dm_wb_encoder_helper_funcs);
> +	wbcon->encoder.possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(dm->adev);
>  
> +	res = drm_encoder_init(&dm->adev->ddev, &wbcon->encoder,
> +			       &amdgpu_dm_wb_encoder_funcs,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
>  	if (res)
>  		return res;
> +
> +	drm_connector_helper_add(&wbcon->base.base, &amdgpu_dm_wb_conn_helper_funcs);
> +
> +	res = drm_writeback_connector_init_with_encoder(&dm->adev->ddev, &wbcon->base,
> +							&wbcon->encoder,
> +							&amdgpu_dm_wb_connector_funcs,
> +							amdgpu_dm_wb_formats,
> +							ARRAY_SIZE(amdgpu_dm_wb_formats));
> +	if (res) {
> +		drm_encoder_cleanup(&wbcon->encoder);
> +		return res;
> +	}
>  	/*
>  	 * Some of the properties below require access to state, like bpc.
>  	 * Allocate some default initial connector state with our reset helper.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c
> index e27f6e504395..9d237f2db95c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c
> @@ -4535,7 +4535,7 @@ static void dm_test_clear_writeback_removes_stream(struct kunit *test)
>  static void dm_test_set_writeback_no_pipe(struct kunit *test)
>  {
>  	struct amdgpu_device *adev = dm_kunit_alloc_adev(test);
> -	struct drm_writeback_connector *wb_conn;
> +	struct amdgpu_dm_wb_connector *wb_conn;
>  	struct drm_connector_state *conn_state;
>  	struct drm_writeback_job *job;
>  	struct amdgpu_framebuffer *afb;
> @@ -4565,7 +4565,7 @@ static void dm_test_set_writeback_no_pipe(struct kunit *test)
>  	conn_state->writeback_job = job;
>  	crtc_state->stream = dm_kunit_alloc_stream(test, NULL);
>  
> -	dm_set_writeback(&adev->dm, crtc_state, &wb_conn->base, conn_state);
> +	dm_set_writeback(&adev->dm, crtc_state, &wb_conn->base.base, conn_state);
>  
>  	KUNIT_EXPECT_NULL(test, acrtc->wb_conn);
>  	KUNIT_EXPECT_FALSE(test, acrtc->wb_pending);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_wb_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_wb_test.c
> index 03036f906385..2711696f6b9c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_wb_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_wb_test.c
> @@ -506,8 +506,8 @@ static void dm_test_wb_connector_init_success(struct kunit *test)
>  	KUNIT_EXPECT_TRUE(test, wbcon->base.base.funcs != NULL);
>  	KUNIT_EXPECT_TRUE(test, wbcon->base.base.helper_private != NULL);
>  	KUNIT_EXPECT_TRUE(test, wbcon->base.base.state != NULL);
> -	KUNIT_EXPECT_TRUE(test, wbcon->base.encoder.funcs != NULL);
> -	KUNIT_EXPECT_EQ(test, wbcon->base.encoder.possible_crtcs, 0x1);
> +	KUNIT_EXPECT_TRUE(test, wbcon->encoder.funcs != NULL);
> +	KUNIT_EXPECT_EQ(test, wbcon->encoder.possible_crtcs, 0x1);
>  }
>  
>  /* Tests for amdgpu_dm_wb_prepare_job / amdgpu_dm_wb_cleanup_job */