lkml.org 
[lkml]   [2018]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [PATCH v1 2/7] drm: move DRM_SWITCH_POWER defines to drm_device.h
Hi Daniel.

> > +/* Used by drm_device.switch_power_state */
> > +#define DRM_SWITCH_POWER_ON 0
> > +#define DRM_SWITCH_POWER_OFF 1
> > +#define DRM_SWITCH_POWER_CHANGING 2
> > +#define DRM_SWITCH_POWER_DYNAMIC_OFF 3
>
> Since this isn't uapi it'd be nice to change it to an enum, which we can
> then properly kernel-doc and make your references links in the resulting
> html. Otherwise lgtm.
>
> Would need an include stanza for drm_device.h in drm-internals.rst, plus a
> bit of kernel-doc cleanup in here I think (which iirc is why I didn't yet
> do this).

Converting to enum was easy, the documentation part not so.
I have tried to add some documentation based on what I could figure out.
There are room for improvements.

The other task was to include drm_device in the documentation.

This work resulted in the following two patches that I will post
as part of an updated series later.
Posted here to maybe get some initial feedback.

Sam

From 3bc5d6a11a1e04f20a465e2690583c87cee74ac0 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@ravnborg.org>
Date: Thu, 27 Dec 2018 23:03:12 +0100
Subject: [PATCH 1/7] drm: add drm_device.h to kernel-doc

Updated comment style to kernel-doc format

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
Documentation/gpu/drm-internals.rst | 4 +
include/drm/drm_device.h | 157 ++++++++++++++++++++++--------------
2 files changed, 101 insertions(+), 60 deletions(-)

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst
index 5ee9674fb9e9..7a677b2b0ebc 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -149,6 +149,10 @@ Device Instance and Driver Handling
.. kernel-doc:: drivers/gpu/drm/drm_drv.c
:export:

+DRM Device
+----------
+.. kernel-doc:: include/drm/drm_device.h
+
Driver Load
-----------

diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 42411b3ea0c8..cd385d3fc979 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -25,24 +25,39 @@ struct pci_dev;
struct pci_controller;

/**
- * DRM device structure. This structure represent a complete card that
+ * struct drm_device - DRM device structure
+ *
+ * This structure represent a complete card that
* may contain multiple heads.
*/
struct drm_device {
- struct list_head legacy_dev_list;/**< list of devices per driver for stealth attach cleanup */
- int if_version; /**< Highest interface version set */
-
- /** \name Lifetime Management */
- /*@{ */
- struct kref ref; /**< Object ref-count */
- struct device *dev; /**< Device structure of bus-device */
- struct drm_driver *driver; /**< DRM driver managing the device */
- void *dev_private; /**< DRM driver private data */
- struct drm_minor *primary; /**< Primary node */
- struct drm_minor *render; /**< Render node */
+ /** @legacy_dev_list: List of devices per driver for stealth attach cleanup */
+ struct list_head legacy_dev_list;
+
+ /** @if_version: Highest interface version set */
+ int if_version;
+
+ /** @ref: Object ref-count */
+ struct kref ref;
+
+ /** @dev: Device structure of bus-device */
+ struct device *dev;
+
+ /** @driver: DRM driver managing the device */
+ struct drm_driver *driver;
+
+ /** @dev_private: DRM driver private data */
+ void *dev_private;
+
+ /** @primary: Primary node */
+ struct drm_minor *primary;
+
+ /** @render: Render node */
+ struct drm_minor *render;
+
bool registered;

- /* currently active master for this device. Protected by master_mutex */
+ /** @master: Currently active master for this device. Protected by master_mutex */
struct drm_master *master;

/**
@@ -63,23 +78,29 @@ struct drm_device {
*/
bool unplugged;

- struct inode *anon_inode; /**< inode for private address-space */
- char *unique; /**< unique name of the device */
- /*@} */
+ /** @anon_inode: inode for private address-space */
+ struct inode *anon_inode;
+
+ /** @unique: Unique name of the device */
+ char *unique;
+
+ /** @struct_mutex: Lock for others (not drm_minor::master and drm_file::is_master) */
+ struct mutex struct_mutex;
+
+ /** @master_mutex: Lock for drm_minor::master and drm_file::is_master */
+ struct mutex master_mutex;
+
+ /** @open_count: Usage counter for outstanding files open, protected by drm_global_mutex. */
+ int open_count;

- /** \name Locks */
- /*@{ */
- struct mutex struct_mutex; /**< For others */
- struct mutex master_mutex; /**< For drm_minor::master and drm_file::is_master */
- /*@} */
+ /** @buf_lock: Lock for drm_device::buf_use and a few other things. */
+ spinlock_t buf_lock;

- /** \name Usage Counters */
- /*@{ */
- int open_count; /**< Outstanding files open, protected by drm_global_mutex. */
- spinlock_t buf_lock; /**< For drm_device::buf_use and a few other things. */
- int buf_use; /**< Buffers in use -- cannot alloc */
- atomic_t buf_alloc; /**< Buffer allocation in progress */
- /*@} */
+ /** @buf_use: Usage counter for buffers in use -- cannot alloc */
+ int buf_use;
+
+ /** @buf_alloc: Buffer allocation in progress */
+ atomic_t buf_alloc;

struct mutex filelist_mutex;
struct list_head filelist;
@@ -105,33 +126,32 @@ struct drm_device {
*/
struct list_head clientlist;

- /** \name Memory management */
- /*@{ */
- struct list_head maplist; /**< Linked list of regions */
- struct drm_open_hash map_hash; /**< User token hash table for maps */
+ /** @maplist: Memory management - linked list of regions */
+ struct list_head maplist;

- /** \name Context handle management */
- /*@{ */
- struct list_head ctxlist; /**< Linked list of context handles */
- struct mutex ctxlist_mutex; /**< For ctxlist */
+ /** @map_hash: Memory management - user token hash table for maps */
+ struct drm_open_hash map_hash;

- struct idr ctx_idr;
+ /** @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;

- struct list_head vmalist; /**< List of vmas (for debugging) */
+ /** @ctx_idr: Context handle management */
+ struct idr ctx_idr;

- /*@} */
+ /** @vmalist: Context handle management - list of vmas (for debugging) */
+ struct list_head vmalist;

- /** \name DMA support */
- /*@{ */
- struct drm_device_dma *dma; /**< Optional pointer for DMA support */
- /*@} */
+ /** @dma: Optional pointer for DMA support */
+ struct drm_device_dma *dma;

- /** \name Context support */
- /*@{ */
+ /** @context_flag: Context swapping flag */
+ __volatile__ long context_flag;

- __volatile__ long context_flag; /**< Context swapping flag */
- int last_context; /**< Last current context */
- /*@} */
+ /** @last_context: Last current context */
+ int last_context;

/**
* @irq_enabled:
@@ -168,7 +188,12 @@ struct drm_device {
*/
struct drm_vblank_crtc *vblank;

- spinlock_t vblank_time_lock; /**< Protects vblank count and time updates during vblank enable/disable */
+ /**
+ * @vblank_time_lock:
+ *
+ * Protects vblank count and time updates during vblank enable/disable
+ */
+ spinlock_t vblank_time_lock;
spinlock_t vbl_lock;

/**
@@ -186,25 +211,33 @@ struct drm_device {
*
* If non-zeor, &drm_crtc_funcs.get_vblank_counter must be set.
*/
- u32 max_vblank_count; /**< size of vblank counter register */
+
+ /** @max_vblank_count: Size of vblank counter register */
+ u32 max_vblank_count;

/**
- * List of events
+ * @vblank_event_list:
+ *
+ * List of vblank events
*/
struct list_head vblank_event_list;
spinlock_t event_lock;

- /*@} */
+ /** @agp: AGP data */
+ struct drm_agp_head *agp;

- struct drm_agp_head *agp; /**< AGP data */
+ /** @pdev: PCI device structure */
+ struct pci_dev *pdev;

- struct pci_dev *pdev; /**< PCI device structure */
#ifdef __alpha__
struct pci_controller *hose;
#endif

- struct drm_sg_mem *sg; /**< Scatter gather memory */
- unsigned int num_crtcs; /**< Number of CRTCs on this device */
+ /** @sg: Scatter gather memory */
+ struct drm_sg_mem *sg;
+
+ /** @num_crtcs: Number of CRTCs on this device */
+ unsigned int num_crtcs;

struct {
int context;
@@ -214,14 +247,18 @@ struct drm_device {
struct drm_local_map *agp_buffer_map;
unsigned int agp_buffer_token;

- struct drm_mode_config mode_config; /**< Current mode config */
+ /** @mode_config: Current mode config */
+ struct drm_mode_config mode_config;

- /** \name GEM information */
- /*@{ */
+ /** @object_name_lock: GEM information */
struct mutex object_name_lock;
+
+ /** @object_name_idr: GEM information */
struct idr object_name_idr;
+
+ /** @vma_offset_manager: GEM information */
struct drm_vma_offset_manager *vma_offset_manager;
- /*@} */
+
int switch_power_state;

/**
--
2.12.0
From a3c94604f8c32a2a48efc79d15fbc66e0e62fd33 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@ravnborg.org>
Date: Wed, 26 Dec 2018 14:21:01 +0100
Subject: [PATCH 2/7] drm: move DRM_SWITCH_POWER defines to drm_device.h

Move DRM_SWITCH_POWER out of drmP.h to allow users
to get rid of the drmP include.

DRM_SWITCH_POWER defines are used in combination
with drm_device.switch_power_state.

Move the DRM_SWITCH_POWER defines to the file where
drm_device.switch_power_state is defined.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
include/drm/drmP.h | 5 -----
include/drm/drm_device.h | 37 ++++++++++++++++++++++++++++++++++++-
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index b6b8436b5123..2ba786820052 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -95,11 +95,6 @@ struct dma_buf_attachment;
struct pci_dev;
struct pci_controller;

-#define DRM_SWITCH_POWER_ON 0
-#define DRM_SWITCH_POWER_OFF 1
-#define DRM_SWITCH_POWER_CHANGING 2
-#define DRM_SWITCH_POWER_DYNAMIC_OFF 3
-
/* returns true if currently okay to sleep */
static inline bool drm_can_sleep(void)
{
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index cd385d3fc979..098bbc2b169e 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -24,6 +24,33 @@ struct inode;
struct pci_dev;
struct pci_controller;

+
+/**
+ * enum drm_switch_power - power state of drm device
+ */
+
+enum switch_power_state {
+ /**
+ * @DRM_SWITCH_POWER_ON: Power state is ON
+ */
+ DRM_SWITCH_POWER_ON = 0,
+
+ /**
+ * @DRM_SWITCH_POWER_OFF: Power state is OFF
+ */
+ DRM_SWITCH_POWER_OFF = 1,
+
+ /**
+ * @DRM_SWITCH_POWER_CHANGING: Power state is changing
+ */
+ DRM_SWITCH_POWER_CHANGING = 2,
+
+ /**
+ * @DRM_SWITCH_POWER_DYNAMIC_OFF: Suspended
+ */
+ DRM_SWITCH_POWER_DYNAMIC_OFF = 3,
+};
+
/**
* struct drm_device - DRM device structure
*
@@ -259,7 +286,15 @@ struct drm_device {
/** @vma_offset_manager: GEM information */
struct drm_vma_offset_manager *vma_offset_manager;

- int switch_power_state;
+ /**
+ * @switch_power_state:
+ *
+ * Power state of the client.
+ * Used by drivers supporting the switcheroo driver.
+ * The state is maintained in the
+ * &vga_switcheroo_client_ops.set_gpu_state callback
+ */
+ enum switch_power_state switch_power_state;

/**
* @fb_helper:
--
2.12.0


\
 
 \ /
  Last update: 2018-12-27 23:18    [W:0.058 / U:5.012 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site