On Fri, Jul 25, 2025 at 10:33:42AM +0530, Suraj Kandpal wrote:
> Write a function that takes a preallocated drm_connector instead of
> using the one allocated inside the drm writeback connector init
> function.

Please start your commit message with describing the problem.

> 
> Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 76 +++++++++++++++++++++++++++++++++
>  include/drm/drm_writeback.h     |  7 +++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 95b8a2e4bda6..fa58eb0dc7bf 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -416,6 +416,82 @@ int drmm_writeback_connector_init(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drmm_writeback_connector_init);
>  
> +/*
> + * drm_writeback_connector_init_with_conn - Initialize a writeback connector 
> with
> + * custom encoder and connector
> + *
> + * @enc: handle to the already initialized drm encoder
> + * @con_funcs: Connector funcs vtable
> + * @formats: Array of supported pixel formats for the writeback engine
> + * @n_formats: Length of the formats array
> + *
> + * This function assumes that the drm_writeback_connector's encoder has 
> already been
> + * created and initialized before invoking this function.
> + *
> + * In addition, this function also assumes that callers of this API will 
> manage
> + * assigning the encoder helper functions, possible_crtcs and any other 
> encoder
> + * specific operation.

Why?

> + *
> + * Drivers should always use this function instead of drm_connector_init() to
> + * set up writeback connectors if they want to manage themselves the 
> lifetime of the
> + * associated encoder.
> + *
> + * Returns: 0 on success, or a negative error code
> + */
> +int
> +drm_writeback_connector_init_with_conn(struct drm_device *dev, struct 
> drm_connector *connector,
> +                                    struct drm_writeback_connector 
> *wb_connector,
> +                                    struct drm_encoder *enc,
> +                                    const struct drm_connector_funcs 
> *con_funcs,
> +                                    const u32 *formats, int n_formats)
> +{
> +     struct drm_property_blob *blob;
> +     struct drm_mode_config *config = &dev->mode_config;
> +     int ret = create_writeback_properties(dev);
> +
> +     if (ret != 0)
> +             return ret;
> +
> +     blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
> +                                     formats);
> +     if (IS_ERR(blob))
> +             return PTR_ERR(blob);
> +
> +     connector->interlace_allowed = 0;

This function contans a lot of copy-paste from
__drm_writeback_connector_init(), which is obviously a no-go.

> +
> +     ret = drm_connector_init(dev, connector, con_funcs,
> +                              DRM_MODE_CONNECTOR_WRITEBACK);
> +     if (ret)
> +             goto connector_fail;
> +
> +     INIT_LIST_HEAD(&wb_connector->job_queue);
> +     spin_lock_init(&wb_connector->job_lock);
> +
> +     wb_connector->fence_context = dma_fence_context_alloc(1);
> +     spin_lock_init(&wb_connector->fence_lock);
> +     snprintf(wb_connector->timeline_name,
> +              sizeof(wb_connector->timeline_name),
> +              "CONNECTOR:%d-%s", connector->base.id, connector->name);
> +
> +     drm_object_attach_property(&connector->base,
> +                                config->writeback_out_fence_ptr_property, 0);
> +
> +     drm_object_attach_property(&connector->base,
> +                                config->writeback_fb_id_property, 0);
> +
> +     drm_object_attach_property(&connector->base,
> +                                config->writeback_pixel_formats_property,
> +                                blob->base.id);
> +     wb_connector->pixel_formats_blob_ptr = blob;
> +
> +     return 0;
> +
> +connector_fail:
> +     drm_property_blob_put(blob);
> +     return ret;
> +}
> +EXPORT_SYMBOL(drm_writeback_connector_init_with_conn);
> +
>  int drm_writeback_set_fb(struct drm_connector_state *conn_state,
>                        struct drm_framebuffer *fb)
>  {
> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> index c380a7b8f55a..149744dbeef0 100644
> --- a/include/drm/drm_writeback.h
> +++ b/include/drm/drm_writeback.h
> @@ -167,6 +167,13 @@ int drmm_writeback_connector_init(struct drm_device *dev,
>                                 struct drm_encoder *enc,
>                                 const u32 *formats, int n_formats);
>  
> +int
> +drm_writeback_connector_init_with_conn(struct drm_device *dev, struct 
> drm_connector *connector,
> +                                    struct drm_writeback_connector 
> *wb_connector,
> +                                    struct drm_encoder *enc,
> +                                    const struct drm_connector_funcs 
> *con_funcs,
> +                                    const u32 *formats, int n_formats);
> +
>  int drm_writeback_set_fb(struct drm_connector_state *conn_state,
>                        struct drm_framebuffer *fb);
>  
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Reply via email to