Re: [PATCH 08/64] drm/writeback: Introduce drmm_writeback_connector_init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 10.06.22 um 11:28 schrieb Maxime Ripard:
Let's create a DRM-managed variant of drmm_writeback_connector_init that
will take care of an action of the encoder and connector cleanup.

Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
---
  drivers/gpu/drm/drm_writeback.c | 136 ++++++++++++++++++++++++--------
  include/drm/drm_writeback.h     |   5 ++
  2 files changed, 108 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
index dccf4504f1bb..0b00921f3379 100644
--- a/drivers/gpu/drm/drm_writeback.c
+++ b/drivers/gpu/drm/drm_writeback.c
@@ -149,32 +149,27 @@ static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
  	.destroy = drm_encoder_cleanup,
  };
-/**
- * drm_writeback_connector_init - Initialize a writeback connector and its properties
- * @dev: DRM device
- * @wb_connector: Writeback connector to initialize
- * @con_funcs: Connector funcs vtable
- * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
- * @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. It will also create an internal encoder associated with the
- * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
- * the encoder helper.
- *
- * Drivers should always use this function instead of drm_connector_init() to
- * set up writeback connectors.
- *
- * Returns: 0 on success, or a negative error code
- */
-int drm_writeback_connector_init(struct drm_device *dev,
-				 struct drm_writeback_connector *wb_connector,
-				 const struct drm_connector_funcs *con_funcs,
-				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
-				 const u32 *formats, int n_formats)
+typedef int (*encoder_init_t)(struct drm_device *dev,
+			      struct drm_encoder *encoder,
+			      const struct drm_encoder_funcs *funcs,
+			      int encoder_type,
+			      const char *name, ...);
+
+typedef int (*connector_init_t)(struct drm_device *dev,
+				struct drm_connector *connector,
+				const struct drm_connector_funcs *funcs,
+				int connector_type);

This code has menawhile changed in drm-tip, which makes it harder to give a good review for such refactoring.

But generally, I'd do the connector and encoder initialization in drmm_writeback_connector_init() and give the initialized encoders to an internal helper that does the common init steps. That avoids such indirections with functions pointers.

Best regards
Thomas

+
+static int writeback_init(struct drm_device *dev,
+			  struct drm_writeback_connector *wb_connector,
+			  connector_init_t conn_init,
+			  void (*conn_destroy)(struct drm_connector *connector),
+			  encoder_init_t enc_init,
+			  void (*enc_destroy)(struct drm_encoder *encoder),
+			  const struct drm_connector_funcs *con_funcs,
+			  const struct drm_encoder_funcs *enc_funcs,
+			  const struct drm_encoder_helper_funcs *enc_helper_funcs,
+			  const u32 *formats, int n_formats)
  {
  	struct drm_property_blob *blob;
  	struct drm_connector *connector = &wb_connector->base;
@@ -190,16 +185,16 @@ int drm_writeback_connector_init(struct drm_device *dev,
  		return PTR_ERR(blob);
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);
+	ret = enc_init(dev, &wb_connector->encoder,
+		       enc_funcs,
+		       DRM_MODE_ENCODER_VIRTUAL, NULL);
  	if (ret)
  		goto fail;
connector->interlace_allowed = 0; - ret = drm_connector_init(dev, connector, con_funcs,
-				 DRM_MODE_CONNECTOR_WRITEBACK);
+	ret = conn_init(dev, connector, con_funcs,
+			DRM_MODE_CONNECTOR_WRITEBACK);
  	if (ret)
  		goto connector_fail;
@@ -231,15 +226,90 @@ int drm_writeback_connector_init(struct drm_device *dev,
  	return 0;
attach_fail:
-	drm_connector_cleanup(connector);
+	if (conn_destroy)
+		conn_destroy(connector);
  connector_fail:
-	drm_encoder_cleanup(&wb_connector->encoder);
+	if (enc_destroy)
+		enc_destroy(&wb_connector->encoder);
  fail:
  	drm_property_blob_put(blob);
  	return ret;
  }
+
+/**
+ * drm_writeback_connector_init - Initialize a writeback connector and its properties
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ * @con_funcs: Connector funcs vtable
+ * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
+ * @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. It will also create an internal encoder associated with the
+ * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
+ * the encoder helper.
+ *
+ * Drivers should always use this function instead of drm_connector_init() to
+ * set up writeback connectors.
+ *
+ * Returns: 0 on success, or a negative error code
+ */
+int drm_writeback_connector_init(struct drm_device *dev,
+				 struct drm_writeback_connector *wb_connector,
+				 const struct drm_connector_funcs *con_funcs,
+				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
+				 const u32 *formats, int n_formats)
+{
+	return writeback_init(dev, wb_connector,
+			      drm_connector_init, drm_connector_cleanup,
+			      drm_encoder_init, drm_encoder_cleanup,
+			      con_funcs,
+			      &drm_writeback_encoder_funcs, enc_helper_funcs,
+			      formats, n_formats);
+}
  EXPORT_SYMBOL(drm_writeback_connector_init);
+/**
+ * drmm_writeback_connector_init - Initialize a writeback connector and its properties
+ * @dev: DRM device
+ * @wb_connector: Writeback connector to initialize
+ * @con_funcs: Connector funcs vtable
+ * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
+ * @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. It will also create an internal encoder associated with the
+ * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
+ * the encoder helper.
+ *
+ * The writeback connector is DRM-managed and won't need any cleanup.
+ *
+ * Drivers should always use this function instead of drm_connector_init() to
+ * set up writeback connectors.
+ *
+ * Returns: 0 on success, or a negative error code
+ */
+int drmm_writeback_connector_init(struct drm_device *dev,
+				 struct drm_writeback_connector *wb_connector,
+				 const struct drm_connector_funcs *con_funcs,
+				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
+				 const u32 *formats, int n_formats)
+{
+	return writeback_init(dev, wb_connector,
+			      drmm_connector_init, NULL,
+			      drmm_encoder_init, NULL,
+			      con_funcs,
+			      NULL, enc_helper_funcs,
+			      formats, n_formats);
+}
+EXPORT_SYMBOL(drmm_writeback_connector_init);
+
  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 9697d2714d2a..cc259ae44734 100644
--- a/include/drm/drm_writeback.h
+++ b/include/drm/drm_writeback.h
@@ -151,6 +151,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
  				 const struct drm_connector_funcs *con_funcs,
  				 const struct drm_encoder_helper_funcs *enc_helper_funcs,
  				 const u32 *formats, int n_formats);
+int drmm_writeback_connector_init(struct drm_device *dev,
+				  struct drm_writeback_connector *wb_connector,
+				  const struct drm_connector_funcs *con_funcs,
+				  const struct drm_encoder_helper_funcs *enc_helper_funcs,
+				  const u32 *formats, int n_formats);
int drm_writeback_set_fb(struct drm_connector_state *conn_state,
  			 struct drm_framebuffer *fb);

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux