Hi Abhinav,
Thank you for the patch.
On Thu, Jan 20, 2022 at 06:29:55PM -0800, Abhinav Kumar wrote:
> Instead of creating an internal encoder for the writeback
> connector to satisfy DRM requirements, allow the clients
> to pass a real encoder to it by changing the drm_writeback's
> encoder to a pointer.
>
> If a real encoder is not passed, drm_writeback_connector_init
> will internally allocate one.
>
> This will help the clients to manage the real encoder states
> better as they will allocate and maintain the encoder.
A writeback connector is a bit of a hack. It was implemented that way to
minimize the extensions to the KMS userspace API for writeback support.
There's no "encoder" there, as there's no real "connector" either. The
only reason we register a drm_encoder in the writeback implementation is
because encoders are exposed to userspace and are thus required (this is
considered a historical mistake that we can't fix anymore). Why do you
thus need to create a "real encoder" ?
> Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_writeback.c | 11 +++++++----
> include/drm/drm_writeback.h | 2 +-
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index dccf4504..fdb7381 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -189,8 +189,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
> if (IS_ERR(blob))
> return PTR_ERR(blob);
>
> - drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> - ret = drm_encoder_init(dev, &wb_connector->encoder,
> + /* allocate the internal drm encoder if a real one wasnt passed */
> + if (!wb_connector->encoder)
> + wb_connector->encoder = devm_kzalloc(dev->dev, sizeof(struct drm_encoder), GFP_KERNEL);
> + drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> + ret = drm_encoder_init(dev, wb_connector->encoder,
> &drm_writeback_encoder_funcs,
> DRM_MODE_ENCODER_VIRTUAL, NULL);
> if (ret)
> @@ -204,7 +207,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
> goto connector_fail;
>
> ret = drm_connector_attach_encoder(connector,
> - &wb_connector->encoder);
> + wb_connector->encoder);
> if (ret)
> goto attach_fail;
>
> @@ -233,7 +236,7 @@ int drm_writeback_connector_init(struct drm_device *dev,
> attach_fail:
> drm_connector_cleanup(connector);
> connector_fail:
> - drm_encoder_cleanup(&wb_connector->encoder);
> + drm_encoder_cleanup(wb_connector->encoder);
> fail:
> drm_property_blob_put(blob);
> return ret;
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index 9697d27..f0d8147 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -31,7 +31,7 @@ struct drm_writeback_connector {
> * by passing the @enc_funcs parameter to drm_writeback_connector_init()
> * function.
> */
> - struct drm_encoder encoder;
> + struct drm_encoder *encoder;
>
> /**
> * @pixel_formats_blob_ptr:
--
Regards,
Laurent Pinchart
