[PATCH 1/5] drm/docs: improve docs for drm_drv.c

[PATCH 1/5] drm/docs: improve docs for drm_drv.c

[Daniel Vetter]

Just a bit of drive-by reading:
- drm_dev_set_unique() is really the exception, make that clear.
- drm_dev_init() is the recomended approach.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_drv.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index a5fe91b8c3c9..381581b01d48 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -264,14 +264,13 @@ void drm_minor_release(struct drm_minor *minor)
  * DOC: driver instance overview
  *
  * A device instance for a drm driver is represented by &struct drm_device. This
- * is allocated with drm_dev_alloc(), usually from bus-specific ->probe()
+ * is initialized with drm_dev_init(), usually from bus-specific ->probe()
  * callbacks implemented by the driver. The driver then needs to initialize all
  * the various subsystems for the drm device like memory management, vblank
  * handling, modesetting support and intial output configuration plus obviously
- * initialize all the corresponding hardware bits. An important part of this is
- * also calling drm_dev_set_unique() to set the userspace-visible unique name of
- * this device instance. Finally when everything is up and running and ready for
- * userspace the device instance can be published using drm_dev_register().
+ * initialize all the corresponding hardware bits. Finally when everything is up
+ * and running and ready for userspace the device instance can be published
+ * using drm_dev_register().
  *
  * There is also deprecated support for initalizing device instances using
  * bus-specific helpers and the &drm_driver.load callback. But due to
@@ -287,9 +286,6 @@ void drm_minor_release(struct drm_minor *minor)
  * Note that the lifetime rules for &drm_device instance has still a lot of
  * historical baggage. Hence use the reference counting provided by
  * drm_dev_get() and drm_dev_put() only carefully.
- *
- * It is recommended that drivers embed &struct drm_device into their own device
- * structure, which is supported through drm_dev_init().
  */
 
 /**
@@ -475,6 +471,9 @@ static void drm_fs_inode_free(struct inode *inode)
  * The initial ref-count of the object is 1. Use drm_dev_get() and
  * drm_dev_put() to take and drop further ref-counts.
  *
+ * It is recommended that drivers embed &struct drm_device into their own device
+ * structure.
+ *
  * Drivers that do not want to allocate their own device struct
  * embedding &struct drm_device can call drm_dev_alloc() instead. For drivers
  * that do embed &struct drm_device it must be placed first in the overall
@@ -765,7 +764,7 @@ static void remove_compat_control_link(struct drm_device *dev)
  * @flags: Flags passed to the driver's .load() function
  *
  * Register the DRM device @dev with the system, advertise device to user-space
- * and start normal device operation. @dev must be allocated via drm_dev_alloc()
+ * and start normal device operation. @dev must be initialized via drm_dev_init()
  * previously.
  *
  * Never call this twice on any device!
@@ -877,9 +876,9 @@ EXPORT_SYMBOL(drm_dev_unregister);
  * @dev: device of which to set the unique name
  * @name: unique name
  *
- * Sets the unique name of a DRM device using the specified string. Drivers
- * can use this at driver probe time if the unique name of the devices they
- * drive is static.
+ * Sets the unique name of a DRM device using the specified string. This is
+ * already done by drm_dev_init(), drivers should only override the default
+ * unique name for backwards compatibility reasons.
  *
  * Return: 0 on success or a negative error code on failure.
  */
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm/of: Fix kerneldoc

[Daniel Vetter]

I noticed a link that didn't work ...

Fixes: 1f2db3034c9f ("drm: of: introduce drm_of_find_panel_or_bridge")
Cc: Rob Herring <robh@kernel.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_of.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 2763a5ec845b..f2f71d71494a 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -217,9 +217,11 @@ int drm_of_encoder_active_endpoint(struct device_node *node,
 }
 EXPORT_SYMBOL_GPL(drm_of_encoder_active_endpoint);
 
-/*
+/**
  * drm_of_find_panel_or_bridge - return connected panel or bridge device
  * @np: device tree node containing encoder output ports
+ * @port: port in the device tree node
+ * @endpoint: endpoint in the device tree node
  * @panel: pointer to hold returned drm_panel
  * @bridge: pointer to hold returned drm_bridge
  *
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/panel: Small documentation polish

[Daniel Vetter]

Need to make sure people can find the panel-bridge to avoid typing too
much.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/bridge/panel.c | 22 ++++++++++++++++++----
 drivers/gpu/drm/drm_panel.c    |  3 +++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index caf12b8fd572..e3687d78cd0c 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -134,8 +134,8 @@ static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
 };
 
 /**
- * drm_panel_bridge_add - Creates a drm_bridge and drm_connector that
- * just calls the appropriate functions from drm_panel.
+ * drm_panel_bridge_add - Creates a &drm_bridge and &drm_connector that
+ * just calls the appropriate functions from &drm_panel.
  *
  * @panel: The drm_panel being wrapped.  Must be non-NULL.
  * @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
@@ -149,9 +149,12 @@ static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
  * passed to drm_bridge_attach().  The drm_panel_prepare() and related
  * functions can be dropped from the encoder driver (they're now
  * called by the KMS helpers before calling into the encoder), along
- * with connector creation.  When done with the bridge,
- * drm_bridge_detach() should be called as normal, then
+ * with connector creation.  When done with the bridge (after
+ * drm_mode_config_cleanup() if the bridge has already been attached), then
  * drm_panel_bridge_remove() to free it.
+ *
+ * See devm_drm_panel_bridge_add() for an automatically manged version of this
+ * function.
  */
 struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
 					u32 connector_type)
@@ -210,6 +213,17 @@ static void devm_drm_panel_bridge_release(struct device *dev, void *res)
 	drm_panel_bridge_remove(*bridge);
 }
 
+/**
+ * devm_drm_panel_bridge_add - Creates a managed &drm_bridge and &drm_connector
+ * that just calls the appropriate functions from &drm_panel.
+ * @dev: device to tie the bridge lifetime to
+ * @panel: The drm_panel being wrapped.  Must be non-NULL.
+ * @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
+ * created.
+ *
+ * This is the managed version of drm_panel_bridge_add() which automatically
+ * calls drm_panel_bridge_remove() when @dev is unbound.
+ */
 struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
 					     struct drm_panel *panel,
 					     u32 connector_type)
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index c33f95e08e1b..dbd5b873e8f2 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -36,6 +36,9 @@ static LIST_HEAD(panel_list);
  * The DRM panel helpers allow drivers to register panel objects with a
  * central registry and provide functions to retrieve those panels in display
  * drivers.
+ *
+ * For easy integration into drivers using the &drm_bridge infrastructure please
+ * take look at drm_panel_bridge_add() and devm_drm_panel_bridge_add().
  */
 
 /**
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/doc: Move bridge link target to the right place

[Daniel Vetter]

I screwed up a rebase somehow.

v2: Drop bogus hunk.

v3: Really drop bogus hunk (Lubomir).

Cc: Lubomir Rintel <lkundrak@v3.sk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-kms-helpers.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 7a3fc569bc68..fbd11b2fe5b5 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -116,8 +116,6 @@ Framebuffer CMA Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_fb_cma_helper.c
    :export:
 
-.. _drm_bridges:
-
 Framebuffer GEM Helper Reference
 ================================
 
@@ -127,6 +125,8 @@ Framebuffer GEM Helper Reference
 .. kernel-doc:: drivers/gpu/drm/drm_gem_framebuffer_helper.c
    :export:
 
+.. _drm_bridges:
+
 Bridges
 =======
 
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] drm/doc: Polish kerneldoc for drm_device.h

[Daniel Vetter]

- Move all the legacy gunk at the bottom, and exclude it from
  kerneldoc.
- Documentation for the remaining bits.

Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/drm-internals.rst |   3 +
 include/drm/drm_device.h            | 150 ++++++++++++++++------------
 2 files changed, 88 insertions(+), 65 deletions(-)

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index 36c569444e12..2d2d8ad7e05f 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -81,6 +81,9 @@ Device Instance and Driver Handling
 .. kernel-doc:: drivers/gpu/drm/drm_drv.c
    :doc: driver instance overview
 
+.. kernel-doc:: include/drm/drm_device.h
+   :internal:
+
 .. kernel-doc:: include/drm/drm_drv.h
    :internal:
 
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index d7cedbac66a3..ab6d3a6396bb 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -69,7 +69,13 @@ struct drm_device {
 	/** @driver: DRM driver managing the device */
 	struct drm_driver *driver;
 
-	/** @dev_private: DRM driver private data */
+	/**
+	 * @dev_private:
+	 *
+	 * DRM driver private data. Instead of using this pointer it is
+	 * recommended that drivers use drm_dev_init() and embed struct
+	 * &drm_device in their larger per-device structure.
+	 */
 	void *dev_private;
 
 	/** @primary: Primary node */
@@ -78,6 +84,11 @@ struct drm_device {
 	/** @render: Render node */
 	struct drm_minor *render;
 
+	/**
+	 * @registered:
+	 *
+	 * Internally used by drm_dev_register() and drm_connector_register().
+	 */
 	bool registered;
 
 	/**
@@ -134,16 +145,13 @@ struct drm_device {
 	 */
 	int open_count;
 
-	/** @buf_lock: Lock for &buf_use and a few other things. */
-	spinlock_t buf_lock;
-
-	/** @buf_use: Usage counter for buffers in use -- cannot alloc */
-	int buf_use;
-
-	/** @buf_alloc: Buffer allocation in progress */
-	atomic_t buf_alloc;
-
+	/** @filelist_mutex: Protects @filelist. */
 	struct mutex filelist_mutex;
+	/**
+	 * @filelist:
+	 *
+	 * List of userspace clients, linked through &drm_file.lhead.
+	 */
 	struct list_head filelist;
 
 	/**
@@ -168,46 +176,6 @@ struct drm_device {
 	 */
 	struct list_head clientlist;
 
-	/** @maplist: Memory management - linked list of regions */
-	struct list_head maplist;
-
-	/** @map_hash: Memory management - user token hash table for maps */
-	struct drm_open_hash map_hash;
-
-	/**
-	 * @ctxlist:
-	 * Context handle management - linked list of context handles
-	 */
-	struct list_head ctxlist;
-
-	/**
-	 * @ctxlist_mutex:
-	 *
-	 * Context handle management - mutex for &ctxlist
-	 */
-	struct mutex ctxlist_mutex;
-
-	/**
-	 * @ctx_idr:
-	 * Context handle management
-	 */
-	struct idr ctx_idr;
-
-	/**
-	 * @vmalist:
-	 * Context handle management - list of vmas (for debugging)
-	 */
-	struct list_head vmalist;
-
-	/** @dma: Optional pointer for DMA support */
-	struct drm_device_dma *dma;
-
-	/** @context_flag: Context swapping flag */
-	__volatile__ long context_flag;
-
-	/** @last_context: Last current context */
-	int last_context;
-
 	/**
 	 * @irq_enabled:
 	 *
@@ -216,6 +184,10 @@ struct drm_device {
 	 * to true manually.
 	 */
 	bool irq_enabled;
+
+	/**
+	 * @irq: Use by the drm_irq_install() and drm_irq_unistall() helpers.
+	 */
 	int irq;
 
 	/**
@@ -249,6 +221,10 @@ struct drm_device {
 	 *  Protects vblank count and time updates during vblank enable/disable
 	 */
 	spinlock_t vblank_time_lock;
+	/**
+	 * @vbl_lock: Top-level vblank references lock, wraps the low-level
+	 * @vblank_time_lock.
+	 */
 	spinlock_t vbl_lock;
 
 	/**
@@ -264,14 +240,19 @@ struct drm_device {
 	 * races and imprecision over longer time periods, hence exposing a
 	 * hardware vblank counter is always recommended.
 	 *
-	 * If non-zeor, &drm_crtc_funcs.get_vblank_counter must be set.
+	 * If non-zero, &drm_crtc_funcs.get_vblank_counter must be set.
 	 */
-
-	/** @max_vblank_count: Size of vblank counter register */
 	u32 max_vblank_count;
 
 	/** @vblank_event_list: List of vblank events */
 	struct list_head vblank_event_list;
+
+	/**
+	 * @event_lock:
+	 *
+	 * Protects @vblank_event_list and event delivery in
+	 * general. See drm_send_event() and drm_send_event_locked().
+	 */
 	spinlock_t event_lock;
 
 	/** @agp: AGP data */
@@ -281,23 +262,12 @@ struct drm_device {
 	struct pci_dev *pdev;
 
 #ifdef __alpha__
+	/** @hose: PCI hose, only used on ALPHA platforms. */
 	struct pci_controller *hose;
 #endif
-
-	/** @sg: Scatter gather memory */
-	struct drm_sg_mem *sg;
-
 	/** @num_crtcs: Number of CRTCs on this device */
 	unsigned int num_crtcs;
 
-	struct {
-		int context;
-		struct drm_hw_lock *lock;
-	} sigdata;
-
-	struct drm_local_map *agp_buffer_map;
-	unsigned int agp_buffer_token;
-
 	/** @mode_config: Current mode config */
 	struct drm_mode_config mode_config;
 
@@ -327,6 +297,56 @@ struct drm_device {
 	 * Set by drm_fb_helper_init() and cleared by drm_fb_helper_fini().
 	 */
 	struct drm_fb_helper *fb_helper;
+
+	/* Everything below here is for legacy driver, never use! */
+	/* private: */
+
+	/* Context handle management - linked list of context handles */
+	struct list_head ctxlist;
+
+	/* Context handle management - mutex for &ctxlist */
+	struct mutex ctxlist_mutex;
+
+	/* Context handle management */
+	struct idr ctx_idr;
+
+	/* Memory management - linked list of regions */
+	struct list_head maplist;
+
+	/* Memory management - user token hash table for maps */
+	struct drm_open_hash map_hash;
+
+	/* Context handle management - list of vmas (for debugging) */
+	struct list_head vmalist;
+
+	/* Optional pointer for DMA support */
+	struct drm_device_dma *dma;
+
+	/* Context swapping flag */
+	__volatile__ long context_flag;
+
+	/* Last current context */
+	int last_context;
+
+	/* Lock for &buf_use and a few other things. */
+	spinlock_t buf_lock;
+
+	/* Usage counter for buffers in use -- cannot alloc */
+	int buf_use;
+
+	/* Buffer allocation in progress */
+	atomic_t buf_alloc;
+
+	struct {
+		int context;
+		struct drm_hw_lock *lock;
+	} sigdata;
+
+	struct drm_local_map *agp_buffer_map;
+	unsigned int agp_buffer_token;
+
+	/* Scatter gather memory */
+	struct drm_sg_mem *sg;
 };
 
 #endif
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/doc: Polish kerneldoc for drm_device.h

[Sam Ravnborg]

Hi Daniel.

Thanks, good to have it ready and plugged into DRM doc.
I like that the legacy stuff is now separate - so one knows
what not to look at.

> +
> +	/**
> +	 * @irq: Use by the drm_irq_install() and drm_irq_unistall() helpers.
> +	 */
>  	int irq;

"Use => Used"

Other than the above nit:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] drm/doc: Polish kerneldoc for drm_device.h

[Daniel Vetter]

On Fri, Jan 11, 2019 at 06:15:17PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
> 
> Thanks, good to have it ready and plugged into DRM doc.
> I like that the legacy stuff is now separate - so one knows
> what not to look at.
> 
> > +
> > +	/**
> > +	 * @irq: Use by the drm_irq_install() and drm_irq_unistall() helpers.
> > +	 */
> >  	int irq;
> 
> "Use => Used"
> 
> Other than the above nit:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

Fixed and applied, thanks for taking a look. Care to also review the first
4 patches in this small documentation series?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm/docs: improve docs for drm_drv.c

[Sam Ravnborg]

On Fri, Jan 11, 2019 at 05:40:44PM +0100, Daniel Vetter wrote:
> Just a bit of drive-by reading:
> - drm_dev_set_unique() is really the exception, make that clear.
> - drm_dev_init() is the recomended approach.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/of: Fix kerneldoc

[Sam Ravnborg]

On Fri, Jan 11, 2019 at 05:40:45PM +0100, Daniel Vetter wrote:
> I noticed a link that didn't work ...
> 
> Fixes: 1f2db3034c9f ("drm: of: introduce drm_of_find_panel_or_bridge")
> Cc: Rob Herring <robh@kernel.org>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm/panel: Small documentation polish

[Sam Ravnborg]

On Fri, Jan 11, 2019 at 05:40:46PM +0100, Daniel Vetter wrote:
> Need to make sure people can find the panel-bridge to avoid typing too
> much.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/doc: Move bridge link target to the right place

[Sam Ravnborg]

On Fri, Jan 11, 2019 at 05:40:47PM +0100, Daniel Vetter wrote:
> I screwed up a rebase somehow.
> 
> v2: Drop bogus hunk.
> 
> v3: Really drop bogus hunk (Lubomir).
> 
> Cc: Lubomir Rintel <lkundrak@v3.sk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/doc: Move bridge link target to the right place

[Daniel Vetter]

On Fri, Jan 11, 2019 at 11:36:39PM +0100, Sam Ravnborg wrote:
> On Fri, Jan 11, 2019 at 05:40:47PM +0100, Daniel Vetter wrote:
> > I screwed up a rebase somehow.
> > 
> > v2: Drop bogus hunk.
> > 
> > v3: Really drop bogus hunk (Lubomir).
> > 
> > Cc: Lubomir Rintel <lkundrak@v3.sk>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Thanks a lot for your review, all remaining patches applied to
drm-misc-next.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel