As the old drm and the new drmm variants of drm_writeback_connector
requires almost the same initialization, create an internal helper to do
most of the initialization work.
Currently there is no cleanup function for writeback connectors. To allows
implementation of drmm variant of writeback connector, create a cleanup
function that can be used to properly remove all the writeback-specific
properties and allocations.
This also introduce an helper to cleanup only the drm_writeback_connector
properties, so it can be used during initialization to cleanup in case of
failure.
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
drivers/gpu/drm/drm_writeback.c | 130 ++++++++++++++++++++++++++++++++--------
1 file changed, 104 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index 33a3c98a962d1ec49ac4b353902036cf74290ae6..494400b09796d37ed89145da45d5f1e029632de5 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -15,6 +15,7 @@
#include <drm/drm_device.h>
#include <drm/drm_drv.h>
#include <drm/drm_framebuffer.h>
+#include <drm/drm_managed.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_property.h>
#include <drm/drm_writeback.h>
@@ -140,6 +141,22 @@ 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;
+ }
+}
+
static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
@@ -202,7 +219,6 @@ EXPORT_SYMBOL(drm_writeback_connector_init);
* @dev: DRM device
* @wb_connector: Writeback connector to initialize
* @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
*
@@ -218,41 +234,31 @@ EXPORT_SYMBOL(drm_writeback_connector_init);
* assigning the encoder helper functions, possible_crtcs and any other encoder
* specific operation.
*
- * 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_encoder(struct drm_device *dev,
- struct drm_writeback_connector *wb_connector, struct drm_encoder *enc,
- const struct drm_connector_funcs *con_funcs, const u32 *formats,
- int n_formats)
+static int __drm_writeback_connector_init(struct drm_device *dev,
+ struct drm_writeback_connector *wb_connector,
+ struct drm_encoder *enc, const u32 *formats,
+ int n_formats)
{
- struct drm_property_blob *blob;
struct drm_connector *connector = &wb_connector->base;
struct drm_mode_config *config = &dev->mode_config;
+ struct drm_property_blob *blob;
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;
- ret = drm_connector_init(dev, connector, con_funcs,
- DRM_MODE_CONNECTOR_WRITEBACK);
- if (ret)
- goto connector_fail;
-
ret = drm_connector_attach_encoder(connector, enc);
if (ret)
- goto attach_fail;
+ return ret;
+
+ blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
+ formats);
+ if (IS_ERR(blob))
+ return PTR_ERR(blob);
INIT_LIST_HEAD(&wb_connector->job_queue);
spin_lock_init(&wb_connector->job_lock);
@@ -275,15 +281,87 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
wb_connector->pixel_formats_blob_ptr = blob;
return 0;
+}
+
+/**
+ * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
+ * a custom encoder
+ *
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ * @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 creates the writeback-connector-specific properties if they
+ * have not been already created, initializes the connector as
+ * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
+ * values.
+ *
+ * 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.
+ *
+ * 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_encoder(struct drm_device *dev,
+ 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_connector *connector = &wb_connector->base;
+ int ret;
+
+ ret = drm_connector_init(dev, connector, con_funcs,
+ DRM_MODE_CONNECTOR_WRITEBACK);
+ if (ret)
+ return ret;
+
+ ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats,
+ n_formats);
+ if (ret)
+ drm_connector_cleanup(connector);
-attach_fail:
- drm_connector_cleanup(connector);
-connector_fail:
- drm_property_blob_put(blob);
return ret;
}
EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
+/**
+ * drm_writeback_connector_cleanup - Cleanup the writeback connector
+ * @dev: DRM device
+ * @wb_connector: 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
+ * clean up the attached encoder and the drm_connector.
+ */
+static void drm_writeback_connector_cleanup(struct drm_device *dev,
+ struct drm_writeback_connector *wb_connector)
+{
+ unsigned long flags;
+ struct drm_writeback_job *pos, *n;
+
+ delete_writeback_properties(dev);
+ drm_property_blob_put(wb_connector->pixel_formats_blob_ptr);
+
+ spin_lock_irqsave(&wb_connector->job_lock, flags);
+ list_for_each_entry_safe(pos, n, &wb_connector->job_queue, list_entry) {
+ drm_writeback_cleanup_job(pos);
+ list_del(&pos->list_entry);
+ }
+ spin_unlock_irqrestore(&wb_connector->job_lock, flags);
+}
+
int drm_writeback_set_fb(struct drm_connector_state *conn_state,
struct drm_framebuffer *fb)
{
--
2.47.1Hi Louis,
kernel test robot noticed the following build warnings:
[auto build test WARNING on f8a2397baf041a5cee408b082334bb09c7e161df]
url: https://github.com/intel-lab-lkp/linux/commits/Louis-Chauvet/drm-vkms-Switch-to-managed-for-connector/20250114-011112
base: f8a2397baf041a5cee408b082334bb09c7e161df
patch link: https://lore.kernel.org/r/20250113-google-vkms-managed-v7-4-4f81d1893e0b%40bootlin.com
patch subject: [PATCH v7 4/7] drm: writeback: Create an helper for drm_writeback_connector initialization
config: i386-randconfig-014-20250115 (https://download.01.org/0day-ci/archive/20250115/202501150938.a3MspUwj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250115/202501150938.a3MspUwj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501150938.a3MspUwj-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/drm_writeback.c: In function 'drm_writeback_connector_init_with_encoder':
>> drivers/gpu/drm/drm_writeback.c:321:35: warning: unused variable 'blob' [-Wunused-variable]
321 | struct drm_property_blob *blob;
| ^~~~
drivers/gpu/drm/drm_writeback.c: At top level:
drivers/gpu/drm/drm_writeback.c:348:13: warning: 'drm_writeback_connector_cleanup' defined but not used [-Wunused-function]
348 | static void drm_writeback_connector_cleanup(struct drm_device *dev,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
>> drivers/gpu/drm/drm_writeback.c:243: warning: expecting prototype for drm_writeback_connector_init_with_encoder(). Prototype was for __drm_writeback_connector_init() instead
vim +/blob +321 drivers/gpu/drm/drm_writeback.c
214
215 /**
216 * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
217 * a custom encoder
218 *
219 * @dev: DRM device
220 * @wb_connector: Writeback connector to initialize
221 * @enc: handle to the already initialized drm encoder
222 * @formats: Array of supported pixel formats for the writeback engine
223 * @n_formats: Length of the formats array
224 *
225 * This function creates the writeback-connector-specific properties if they
226 * have not been already created, initializes the connector as
227 * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
228 * values.
229 *
230 * This function assumes that the drm_writeback_connector's encoder has already been
231 * created and initialized before invoking this function.
232 *
233 * In addition, this function also assumes that callers of this API will manage
234 * assigning the encoder helper functions, possible_crtcs and any other encoder
235 * specific operation.
236 *
237 * Returns: 0 on success, or a negative error code
238 */
239 static int __drm_writeback_connector_init(struct drm_device *dev,
240 struct drm_writeback_connector *wb_connector,
241 struct drm_encoder *enc, const u32 *formats,
242 int n_formats)
> 243 {
244 struct drm_connector *connector = &wb_connector->base;
245 struct drm_mode_config *config = &dev->mode_config;
246 struct drm_property_blob *blob;
247 int ret = create_writeback_properties(dev);
248
249 if (ret != 0)
250 return ret;
251
252 connector->interlace_allowed = 0;
253
254 ret = drm_connector_attach_encoder(connector, enc);
255 if (ret)
256 return ret;
257
258 blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
259 formats);
260 if (IS_ERR(blob))
261 return PTR_ERR(blob);
262
263 INIT_LIST_HEAD(&wb_connector->job_queue);
264 spin_lock_init(&wb_connector->job_lock);
265
266 wb_connector->fence_context = dma_fence_context_alloc(1);
267 spin_lock_init(&wb_connector->fence_lock);
268 snprintf(wb_connector->timeline_name,
269 sizeof(wb_connector->timeline_name),
270 "CONNECTOR:%d-%s", connector->base.id, connector->name);
271
272 drm_object_attach_property(&connector->base,
273 config->writeback_out_fence_ptr_property, 0);
274
275 drm_object_attach_property(&connector->base,
276 config->writeback_fb_id_property, 0);
277
278 drm_object_attach_property(&connector->base,
279 config->writeback_pixel_formats_property,
280 blob->base.id);
281 wb_connector->pixel_formats_blob_ptr = blob;
282
283 return 0;
284 }
285
286 /**
287 * drm_writeback_connector_init_with_encoder - Initialize a writeback connector with
288 * a custom encoder
289 *
290 * @dev: DRM device
291 * @wb_connector: Writeback connector to initialize
292 * @enc: handle to the already initialized drm encoder
293 * @con_funcs: Connector funcs vtable
294 * @formats: Array of supported pixel formats for the writeback engine
295 * @n_formats: Length of the formats array
296 *
297 * This function creates the writeback-connector-specific properties if they
298 * have not been already created, initializes the connector as
299 * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
300 * values.
301 *
302 * This function assumes that the drm_writeback_connector's encoder has already been
303 * created and initialized before invoking this function.
304 *
305 * In addition, this function also assumes that callers of this API will manage
306 * assigning the encoder helper functions, possible_crtcs and any other encoder
307 * specific operation.
308 *
309 * Drivers should always use this function instead of drm_connector_init() to
310 * set up writeback connectors if they want to manage themselves the lifetime of the
311 * associated encoder.
312 *
313 * Returns: 0 on success, or a negative error code
314 */
315 int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
316 struct drm_writeback_connector *wb_connector,
317 struct drm_encoder *enc,
318 const struct drm_connector_funcs *con_funcs,
319 const u32 *formats, int n_formats)
320 {
> 321 struct drm_property_blob *blob;
322 struct drm_connector *connector = &wb_connector->base;
323 int ret;
324
325 ret = drm_connector_init(dev, connector, con_funcs,
326 DRM_MODE_CONNECTOR_WRITEBACK);
327 if (ret)
328 return ret;
329
330 ret = __drm_writeback_connector_init(dev, wb_connector, enc, formats,
331 n_formats);
332 if (ret)
333 drm_connector_cleanup(connector);
334
335 return ret;
336 }
337 EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
338
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.