Re: [PATCH v7 7/9] drm: writeback: don't destroy the shared writeback properties

[Chaoyi Chen <chaoyi.chen@rock-chips.com>]

Hi Dmitry,

On 10/19/2026 9:12 PM, Dmitry Baryshkov wrote:
> The writeback properties are created once per device and attached to
> every writeback connector. Nevertheless, a failing
> __drm_writeback_connector_init() destroys all three of them. If an
> earlier writeback connector has already been registered (komeda and
> rcar-du create one per CRTC), it is left holding stale property
> pointers. The drmm cleanup action likewise destroys them as soon as the
> first connector goes away.
> 
> Leave the properties alone. They are on the device's property list, so
> drm_mode_config_cleanup() destroys them together with all other
> properties.
> 

If a user attaches a private property to a connector, are they 
responsible for calling put on that property during cleanup? 
In that case, can drmm* functions be used to clean up these 
properties? Thank you.


> Fixes: 2f3f4a73631b ("drm: writeback: Add missing cleanup in case of initialization failure")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/drm_writeback.c | 34 ++++++----------------------------
>  1 file changed, 6 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> index 89c36f1414ce..2f6a7b50336c 100644
> --- a/drivers/gpu/drm/drm_writeback.c
> +++ b/drivers/gpu/drm/drm_writeback.c
> @@ -148,22 +148,6 @@ static int create_writeback_properties(struct drm_device *dev)
>  	return 0;
>  }
>  
> -static void delete_writeback_properties(struct drm_device *dev)
> -{
> -	if (dev->mode_config.writeback_pixel_formats_property) {
> -		drm_property_destroy(dev, dev->mode_config.writeback_pixel_formats_property);
> -		dev->mode_config.writeback_pixel_formats_property = NULL;
> -	}
> -	if (dev->mode_config.writeback_out_fence_ptr_property) {
> -		drm_property_destroy(dev, dev->mode_config.writeback_out_fence_ptr_property);
> -		dev->mode_config.writeback_out_fence_ptr_property = NULL;
> -	}
> -	if (dev->mode_config.writeback_fb_id_property) {
> -		drm_property_destroy(dev, dev->mode_config.writeback_fb_id_property);
> -		dev->mode_config.writeback_fb_id_property = NULL;
> -	}
> -}
> -
>  /**
>   * __drm_writeback_connector_init - Initialize a writeback connector
>   *
> @@ -198,20 +182,18 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>  	int ret = create_writeback_properties(dev);
>  
>  	if (ret != 0)
> -		goto failed_properties;
> +		return ret;
>  
>  	connector->interlace_allowed = 0;
>  
>  	ret = drm_connector_attach_encoder(connector, enc);
>  	if (ret)
> -		goto failed_properties;
> +		return ret;
>  
>  	blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
>  					formats);
> -	if (IS_ERR(blob)) {
> -		ret = PTR_ERR(blob);
> -		goto failed_properties;
> -	}
> +	if (IS_ERR(blob))
> +		return PTR_ERR(blob);
>  
>  	INIT_LIST_HEAD(&wb_connector->job_queue);
>  	spin_lock_init(&wb_connector->job_lock);
> @@ -234,9 +216,6 @@ static int __drm_writeback_connector_init(struct drm_device *dev,
>  	wb_connector->pixel_formats_blob_ptr = blob;
>  
>  	return 0;
> -failed_properties:
> -	delete_writeback_properties(dev);
> -	return ret;
>  }
>  
>  /**
> @@ -295,8 +274,8 @@ EXPORT_SYMBOL(drm_writeback_connector_init);
>   * @dev: DRM device
>   * @data: Pointer to the writeback connector to clean up
>   *
> - * This will decrement the reference counter of blobs and destroy properties. It
> - * will also clean the remaining jobs in this writeback connector. Caution: This helper will not
> + * This will decrement the reference counter of blobs. It will also clean the
> + * remaining jobs in this writeback connector. Caution: This helper will not
>   * clean up the attached encoder and the drm_connector.
>   */
>  static void drm_writeback_connector_cleanup(struct drm_device *dev,
> @@ -306,7 +285,6 @@ static void drm_writeback_connector_cleanup(struct drm_device *dev,
>  	struct drm_writeback_job *pos, *n;
>  	struct drm_writeback_connector *wb_connector = data;
>  
> -	delete_writeback_properties(dev);
>  	drm_property_blob_put(wb_connector->pixel_formats_blob_ptr);
>  
>  	spin_lock_irqsave(&wb_connector->job_lock, flags);
> 

-- 
Best, 
Chaoyi