Re: [PATCH 1/9] drm: Extract drm_encoder.[hc]

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

 




Hi,

On 08/18/2016 02:25 AM, Daniel Vetter wrote:
Same treatment as before. Only hiccup is drm_crtc_mask, which
unfortunately can't be resolved until drm_crtc.h is less of a monster.
Untangle the header loop with a forward delcaration for that static

s/delcaration/declaration

inline.

Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
---
  Documentation/gpu/drm-kms.rst       |   9 ++
  drivers/gpu/drm/Makefile            |   3 +-
  drivers/gpu/drm/drm_crtc.c          | 193 -------------------------------
  drivers/gpu/drm/drm_crtc_internal.h |  10 +-
  drivers/gpu/drm/drm_encoder.c       | 220 ++++++++++++++++++++++++++++++++++++
  include/drm/drm_crtc.h              | 134 +---------------------
  include/drm/drm_encoder.h           | 167 +++++++++++++++++++++++++++
  7 files changed, 407 insertions(+), 329 deletions(-)
  create mode 100644 drivers/gpu/drm/drm_encoder.c
  create mode 100644 include/drm/drm_encoder.h


<snip>

+
+/**
+ * drm_encoder_init - Init a preallocated encoder
+ * @dev: drm device
+ * @encoder: the encoder to init
+ * @funcs: callbacks for this encoder
+ * @encoder_type: user visible type of the encoder
+ * @name: printf style format string for the encoder name, or NULL for default name
+ *
+ * Initialises a preallocated encoder. Encoder should be
+ * subclassed as part of driver encoder objects.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_encoder_init(struct drm_device *dev,
+		      struct drm_encoder *encoder,
+		      const struct drm_encoder_funcs *funcs,
+		      int encoder_type, const char *name, ...)

Alignment with the open parentheses is needed here.

+{
+	int ret;
+
+	drm_modeset_lock_all(dev);
+
+	ret = drm_mode_object_get(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER);
+	if (ret)
+		goto out_unlock;
+
+	encoder->dev = dev;
+	encoder->encoder_type = encoder_type;
+	encoder->funcs = funcs;
+	if (name) {
+		va_list ap;
+
+		va_start(ap, name);
+		encoder->name = kvasprintf(GFP_KERNEL, name, ap);
+		va_end(ap);
+	} else {
+		encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
+					  drm_encoder_enum_list[encoder_type].name,
+					  encoder->base.id);
+	}
+	if (!encoder->name) {
+		ret = -ENOMEM;
+		goto out_put;
+	}
+
+	list_add_tail(&encoder->head, &dev->mode_config.encoder_list);
+	encoder->index = dev->mode_config.num_encoder++;
+
+out_put:
+	if (ret)
+		drm_mode_object_unregister(dev, &encoder->base);
+
+out_unlock:
+	drm_modeset_unlock_all(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_encoder_init);
+
+/**
+ * drm_encoder_cleanup - cleans up an initialised encoder
+ * @encoder: encoder to cleanup
+ *
+ * Cleans up the encoder but doesn't free the object.
+ */
+void drm_encoder_cleanup(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+
+	/* Note that the encoder_list is considered to be static; should we
+	 * remove the drm_encoder at runtime we would have to decrement all
+	 * the indices on the drm_encoder after us in the encoder_list.
+	 */
+
+	drm_modeset_lock_all(dev);
+	drm_mode_object_unregister(dev, &encoder->base);
+	kfree(encoder->name);
+	list_del(&encoder->head);
+	dev->mode_config.num_encoder--;
+	drm_modeset_unlock_all(dev);
+
+	memset(encoder, 0, sizeof(*encoder));
+}
+EXPORT_SYMBOL(drm_encoder_cleanup);
+
+static struct drm_crtc *drm_encoder_get_crtc(struct drm_encoder *encoder)
+{
+	struct drm_connector *connector;
+	struct drm_device *dev = encoder->dev;
+	bool uses_atomic = false;
+
+	/* For atomic drivers only state objects are synchronously updated and
+	 * protected by modeset locks, so check those first. */
+	drm_for_each_connector(connector, dev) {
+		if (!connector->state)
+			continue;
+
+		uses_atomic = true;
+
+		if (connector->state->best_encoder != encoder)
+			continue;
+
+		return connector->state->crtc;
+	}
+
+	/* Don't return stale data (e.g. pending async disable). */
+	if (uses_atomic)
+		return NULL;
+
+	return encoder->crtc;
+}
+
+/**
+ * drm_mode_getencoder - get encoder configuration
+ * @dev: drm device for the ioctl
+ * @data: data pointer for the ioctl
+ * @file_priv: drm file for the ioctl call
+ *
+ * Construct a encoder configuration structure to return to the user.
+ *
+ * Called by the user via ioctl.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_mode_getencoder(struct drm_device *dev, void *data,
+			struct drm_file *file_priv)
+{
+	struct drm_mode_get_encoder *enc_resp = data;
+	struct drm_encoder *encoder;
+	struct drm_crtc *crtc;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	encoder = drm_encoder_find(dev, enc_resp->encoder_id);
+	if (!encoder)
+		return -ENOENT;
+
+	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
+	crtc = drm_encoder_get_crtc(encoder);
+	if (crtc)
+		enc_resp->crtc_id = crtc->base.id;
+	else
+		enc_resp->crtc_id = 0;
+	drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+	enc_resp->encoder_type = encoder->encoder_type;
+	enc_resp->encoder_id = encoder->base.id;
+	enc_resp->possible_crtcs = encoder->possible_crtcs;
+	enc_resp->possible_clones = encoder->possible_clones;
+
+	return 0;
+}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 3fa0275e509f..61d81fb3c8fc 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -40,6 +40,7 @@
  #include <drm/drm_framebuffer.h>
  #include <drm/drm_modes.h>
  #include <drm/drm_connector.h>
+#include <drm/drm_encoder.h>

  struct drm_device;
  struct drm_mode_set;
@@ -662,97 +663,6 @@ struct drm_crtc {
  };

  /**
- * struct drm_encoder_funcs - encoder controls
- *
- * Encoders sit between CRTCs and connectors.
- */
-struct drm_encoder_funcs {
-	/**
-	 * @reset:
-	 *
-	 * Reset encoder hardware and software state to off. This function isn't
-	 * called by the core directly, only through drm_mode_config_reset().
-	 * It's not a helper hook only for historical reasons.
-	 */
-	void (*reset)(struct drm_encoder *encoder);
-
-	/**
-	 * @destroy:
-	 *
-	 * Clean up encoder resources. This is only called at driver unload time
-	 * through drm_mode_config_cleanup() since an encoder cannot be
-	 * hotplugged in DRM.
-	 */
-	void (*destroy)(struct drm_encoder *encoder);
-
-	/**
-	 * @late_register:
-	 *
-	 * This optional hook can be used to register additional userspace
-	 * interfaces attached to the encoder like debugfs interfaces.
-	 * It is called late in the driver load sequence from drm_dev_register().
-	 * Everything added from this callback should be unregistered in
-	 * the early_unregister callback.
-	 *
-	 * Returns:
-	 *
-	 * 0 on success, or a negative error code on failure.
-	 */
-	int (*late_register)(struct drm_encoder *encoder);
-
-	/**
-	 * @early_unregister:
-	 *
-	 * This optional hook should be used to unregister the additional
-	 * userspace interfaces attached to the encoder from
-	 * late_unregister(). It is called from drm_dev_unregister(),
-	 * early in the driver unload sequence to disable userspace access
-	 * before data structures are torndown.
-	 */
-	void (*early_unregister)(struct drm_encoder *encoder);
-};
-
-/**
- * struct drm_encoder - central DRM encoder structure
- * @dev: parent DRM device
- * @head: list management
- * @base: base KMS object
- * @name: human readable name, can be overwritten by the driver
- * @encoder_type: one of the DRM_MODE_ENCODER_<foo> types in drm_mode.h
- * @possible_crtcs: bitmask of potential CRTC bindings
- * @possible_clones: bitmask of potential sibling encoders for cloning
- * @crtc: currently bound CRTC
- * @bridge: bridge associated to the encoder
- * @funcs: control functions
- * @helper_private: mid-layer private data
- *
- * CRTCs drive pixels to encoders, which convert them into signals
- * appropriate for a given connector or set of connectors.
- */
-struct drm_encoder {
-	struct drm_device *dev;
-	struct list_head head;
-
-	struct drm_mode_object base;
-	char *name;
-	int encoder_type;
-
-	/**
-	 * @index: Position inside the mode_config.list, can be used as an array
-	 * index. It is invariant over the lifetime of the encoder.
-	 */
-	unsigned index;
-
-	uint32_t possible_crtcs;
-	uint32_t possible_clones;
-
-	struct drm_crtc *crtc;
-	struct drm_bridge *bridge;
-	const struct drm_encoder_funcs *funcs;
-	const struct drm_encoder_helper_funcs *helper_private;
-};
-
-/**
   * struct drm_plane_state - mutable plane state
   * @plane: backpointer to the plane
   * @crtc: currently bound CRTC, NULL if disabled
@@ -2091,7 +2001,6 @@ struct drm_mode_config {
  		for_each_if ((encoder_mask) & (1 << drm_encoder_index(encoder)))

  #define obj_to_crtc(x) container_of(x, struct drm_crtc, base)
-#define obj_to_encoder(x) container_of(x, struct drm_encoder, base)
  #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
  #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
  #define obj_to_property(x) container_of(x, struct drm_property, base)
@@ -2136,37 +2045,6 @@ static inline uint32_t drm_crtc_mask(struct drm_crtc *crtc)
  	return 1 << drm_crtc_index(crtc);
  }

-extern __printf(5, 6)
-int drm_encoder_init(struct drm_device *dev,
-		     struct drm_encoder *encoder,
-		     const struct drm_encoder_funcs *funcs,
-		     int encoder_type, const char *name, ...);
-
-/**
- * drm_encoder_index - find the index of a registered encoder
- * @encoder: encoder to find index for
- *
- * Given a registered encoder, return the index of that encoder within a DRM
- * device's list of encoders.
- */
-static inline unsigned int drm_encoder_index(struct drm_encoder *encoder)
-{
-	return encoder->index;
-}
-
-/**
- * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
- * @encoder: encoder to test
- * @crtc: crtc to test
- *
- * Return false if @encoder can't be driven by @crtc, true otherwise.
- */
-static inline bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
-				       struct drm_crtc *crtc)
-{
-	return !!(encoder->possible_crtcs & drm_crtc_mask(crtc));
-}
-
  extern __printf(8, 9)
  int drm_universal_plane_init(struct drm_device *dev,
  			     struct drm_plane *plane,
@@ -2202,8 +2080,6 @@ extern void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
  extern int drm_crtc_force_disable(struct drm_crtc *crtc);
  extern int drm_crtc_force_disable_all(struct drm_device *dev);

-extern void drm_encoder_cleanup(struct drm_encoder *encoder);
-
  extern void drm_mode_config_init(struct drm_device *dev);
  extern void drm_mode_config_reset(struct drm_device *dev);
  extern void drm_mode_config_cleanup(struct drm_device *dev);
@@ -2315,14 +2191,6 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
  	return mo ? obj_to_crtc(mo) : NULL;
  }

-static inline struct drm_encoder *drm_encoder_find(struct drm_device *dev,
-	uint32_t id)
-{
-	struct drm_mode_object *mo;
-	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_ENCODER);
-	return mo ? obj_to_encoder(mo) : NULL;
-}
-
  static inline struct drm_property *drm_property_find(struct drm_device *dev,
  		uint32_t id)
  {
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
new file mode 100644
index 000000000000..2712fd1a686b
--- /dev/null
+++ b/include/drm/drm_encoder.h
@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifndef __DRM_ENCODER_H__
+#define __DRM_ENCODER_H__
+
+#include <linux/list.h>
+#include <linux/ctype.h>
+#include <drm/drm_modeset.h>
+
+/**
+ * struct drm_encoder_funcs - encoder controls
+ *
+ * Encoders sit between CRTCs and connectors.
+ */
+struct drm_encoder_funcs {
+	/**
+	 * @reset:
+	 *
+	 * Reset encoder hardware and software state to off. This function isn't
+	 * called by the core directly, only through drm_mode_config_reset().
+	 * It's not a helper hook only for historical reasons.
+	 */
+	void (*reset)(struct drm_encoder *encoder);
+
+	/**
+	 * @destroy:
+	 *
+	 * Clean up encoder resources. This is only called at driver unload time
+	 * through drm_mode_config_cleanup() since an encoder cannot be
+	 * hotplugged in DRM.
+	 */
+	void (*destroy)(struct drm_encoder *encoder);
+
+	/**
+	 * @late_register:
+	 *
+	 * This optional hook can be used to register additional userspace
+	 * interfaces attached to the encoder like debugfs interfaces.
+	 * It is called late in the driver load sequence from drm_dev_register().
+	 * Everything added from this callback should be unregistered in
+	 * the early_unregister callback.
+	 *
+	 * Returns:
+	 *
+	 * 0 on success, or a negative error code on failure.
+	 */
+	int (*late_register)(struct drm_encoder *encoder);
+
+	/**
+	 * @early_unregister:
+	 *
+	 * This optional hook should be used to unregister the additional
+	 * userspace interfaces attached to the encoder from
+	 * late_unregister(). It is called from drm_dev_unregister(),
+	 * early in the driver unload sequence to disable userspace access
+	 * before data structures are torndown.
+	 */
+	void (*early_unregister)(struct drm_encoder *encoder);
+};
+
+/**
+ * struct drm_encoder - central DRM encoder structure
+ * @dev: parent DRM device
+ * @head: list management
+ * @base: base KMS object
+ * @name: human readable name, can be overwritten by the driver
+ * @encoder_type: one of the DRM_MODE_ENCODER_<foo> types in drm_mode.h
+ * @possible_crtcs: bitmask of potential CRTC bindings
+ * @possible_clones: bitmask of potential sibling encoders for cloning
+ * @crtc: currently bound CRTC
+ * @bridge: bridge associated to the encoder
+ * @funcs: control functions
+ * @helper_private: mid-layer private data
+ *
+ * CRTCs drive pixels to encoders, which convert them into signals
+ * appropriate for a given connector or set of connectors.
+ */
+struct drm_encoder {
+	struct drm_device *dev;
+	struct list_head head;
+
+	struct drm_mode_object base;
+	char *name;
+	int encoder_type;
+
+	/**
+	 * @index: Position inside the mode_config.list, can be used as an array
+	 * index. It is invariant over the lifetime of the encoder.
+	 */
+	unsigned index;
+
+	uint32_t possible_crtcs;
+	uint32_t possible_clones;
+
+	struct drm_crtc *crtc;
+	struct drm_bridge *bridge;
+	const struct drm_encoder_funcs *funcs;
+	const struct drm_encoder_helper_funcs *helper_private;
+};
+
+#define obj_to_encoder(x) container_of(x, struct drm_encoder, base)
+
+__printf(5, 6)
+int drm_encoder_init(struct drm_device *dev,
+		     struct drm_encoder *encoder,
+		     const struct drm_encoder_funcs *funcs,
+		     int encoder_type, const char *name, ...);
+
+/**
+ * drm_encoder_index - find the index of a registered encoder
+ * @encoder: encoder to find index for
+ *
+ * Given a registered encoder, return the index of that encoder within a DRM
+ * device's list of encoders.
+ */
+static inline unsigned int drm_encoder_index(struct drm_encoder *encoder)
+{
+	return encoder->index;
+}
+
+/* FIXME: We have an include file mess still, drm_crtc.h needs untangling. */
+static inline uint32_t drm_crtc_mask(struct drm_crtc *crtc);
+
+/**
+ * drm_encoder_crtc_ok - can a given crtc drive a given encoder?
+ * @encoder: encoder to test
+ * @crtc: crtc to test
+ *
+ * Return false if @encoder can't be driven by @crtc, true otherwise.
+ */
+static inline bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
+				       struct drm_crtc *crtc)
+{
+	return !!(encoder->possible_crtcs & drm_crtc_mask(crtc));
+}
+
+static inline struct drm_encoder *drm_encoder_find(struct drm_device *dev,
+	uint32_t id)

and here.

+{
+	struct drm_mode_object *mo;
+	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_ENCODER);

checkpatch --strict asks for a blank line above too. Otherwise:

Reviewed-by: Archit Taneja <architt@xxxxxxxxxxxxxx>

Thanks,
Archit

+	return mo ? obj_to_encoder(mo) : NULL;
+}
+
+void drm_encoder_cleanup(struct drm_encoder *encoder);
+
+#endif


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[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