From ac4f24c213b81ce668574cebaf3e1b2bc92fcff6 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 8 Jan 2019 20:29:28 +0100 Subject: drm: drm_device.h: update comments to kernel-doc style Updated comment style to kernel-doc format in drm_device.h In struct drm_device there are 12 struct members without doc: - registered - filelist_mutex - filelist - irq - vbl_lock - event_lock - hose - sigdata - sigdata.context - sigdata.lock - agp_buffer_map - agp_buffer_token They all need proper documentation, a task left for someone that knows their usage. drm_device is not plugged into Documentation/gpu/drm-internals.rst as this would create a new load of warnings. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Jonathan Corbet Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-2-sam@ravnborg.org --- include/drm/drm_device.h | 199 +++++++++++++++++++++++++++++++---------------- 1 file changed, 134 insertions(+), 65 deletions(-) (limited to 'include') diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 42411b3ea0c8..2b154ead9efc 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -25,24 +25,48 @@ 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 +87,42 @@ 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; + + /** @buf_lock: Lock for &buf_use and a few other things. */ + spinlock_t buf_lock; - /** \name Locks */ - /*@{ */ - struct mutex struct_mutex; /**< For others */ - struct mutex master_mutex; /**< For drm_minor::master and drm_file::is_master */ - /*@} */ + /** @buf_use: Usage counter for buffers in use -- cannot alloc */ + int buf_use; - /** \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_alloc: Buffer allocation in progress */ + atomic_t buf_alloc; struct mutex filelist_mutex; struct list_head filelist; @@ -87,51 +130,64 @@ struct drm_device { /** * @filelist_internal: * - * List of open DRM files for in-kernel clients. Protected by @filelist_mutex. + * List of open DRM files for in-kernel clients. + * Protected by &filelist_mutex. */ struct list_head filelist_internal; /** * @clientlist_mutex: * - * Protects @clientlist access. + * Protects &clientlist access. */ struct mutex clientlist_mutex; /** * @clientlist: * - * List of in-kernel clients. Protected by @clientlist_mutex. + * List of in-kernel clients. Protected by &clientlist_mutex. */ 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; - struct list_head vmalist; /**< List of vmas (for debugging) */ + /** + * @ctxlist_mutex: + * + * Context handle management - mutex for &ctxlist + */ + struct mutex ctxlist_mutex; - /*@} */ + /** + * @ctx_idr: + * Context handle management + */ + struct idr ctx_idr; - /** \name DMA support */ - /*@{ */ - struct drm_device_dma *dma; /**< Optional pointer for DMA support */ - /*@} */ + /** + * @vmalist: + * Context handle management - list of vmas (for debugging) + */ + struct list_head vmalist; + + /** @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 +224,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 +247,29 @@ struct drm_device { * * If non-zeor, &drm_crtc_funcs.get_vblank_counter must be set. */ - u32 max_vblank_count; /**< size of vblank counter register */ - /** - * List of events - */ + /** @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; 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 +279,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; /** -- cgit v1.2.3 From 7af78f406119366fb15a15823f720ebf06bb5652 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 8 Jan 2019 20:29:29 +0100 Subject: 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. Moved to drm_device.h because drm_device.switch_power_state is the only user. Converted to enum and added sparse kerneldoc comments. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-3-sam@ravnborg.org --- include/drm/drmP.h | 5 ----- include/drm/drm_device.h | 29 ++++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 6 deletions(-) (limited to 'include') diff --git a/include/drm/drmP.h b/include/drm/drmP.h index db94ef00940e..9e47c8dc6b87 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -94,11 +94,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 2b154ead9efc..d7cedbac66a3 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -24,6 +24,25 @@ 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 * @@ -291,7 +310,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: -- cgit v1.2.3 From 2513147dce2353eb6d1a947ab543e3758724362d Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 8 Jan 2019 20:29:30 +0100 Subject: drm: make drm_framebuffer.h self contained MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add forward declaration and pull in include file to make drm_framebuffer.h self contained. While add it order include files alphabetically. The use of TASK_COMM_LEN is the reason for including sched.h. I could not see any good way to avoid this dependency, and users of drm_framebuffer.comm already use TASK_COMM_LEN to check for length etc. v2: - Added forward declaration of drm_gem_object (Noralf) - Added ack from Noralf Signed-off-by: Sam Ravnborg Acked-by: Noralf Trønnes Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-4-sam@ravnborg.org --- include/drm/drm_framebuffer.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h index c94acedfb08e..f0b34c977ec5 100644 --- a/include/drm/drm_framebuffer.h +++ b/include/drm/drm_framebuffer.h @@ -23,13 +23,17 @@ #ifndef __DRM_FRAMEBUFFER_H__ #define __DRM_FRAMEBUFFER_H__ -#include #include +#include +#include + #include -struct drm_framebuffer; -struct drm_file; +struct drm_clip_rect; struct drm_device; +struct drm_file; +struct drm_framebuffer; +struct drm_gem_object; /** * struct drm_framebuffer_funcs - framebuffer hooks -- cgit v1.2.3 From 428747ae5cca2ca13983b768513f13cb8896de04 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 8 Jan 2019 20:29:33 +0100 Subject: drm: remove include of drmP.h from bridge/dw_hdmi.h drmP.h is an relic from the days when there was a single header file. To enable the removal of drmP.h from all users drop include of drmP.h from bridge/dw_hdmi.h. A few files relied on the file included in drmP.h - add explicit include statements or forward declarations to these files. Build tested with arm and x86. v2: - prefer forward declarations when possible (Laurent Pinchart) - sort include files (Laurent Pinchart) Signed-off-by: Sam Ravnborg Reviewed-by: Laurent Pinchart Cc: Archit Taneja Cc: Andrzej Hajda Cc: David Airlie Cc: Daniel Vetter Cc: Kieran Bingham Cc: Fabio Estevam Cc: Neil Armstrong Cc: Maxime Ripard Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-7-sam@ravnborg.org --- drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 4 ++++ drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 1 + include/drm/bridge/dw_hdmi.h | 6 ++++-- 3 files changed, 9 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c index 2228689d9a5e..5cbb71a866d5 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c @@ -5,6 +5,10 @@ * Copyright (c) 2017 Renesas Solutions Corp. * Kuninori Morimoto */ + +#include +#include + #include #include diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c index 75490a3e0a2a..790d499daa10 100644 --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c @@ -7,6 +7,7 @@ * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com) */ +#include #include #include diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h index 9c56412bb2cf..9f93895dde88 100644 --- a/include/drm/bridge/dw_hdmi.h +++ b/include/drm/bridge/dw_hdmi.h @@ -10,9 +10,11 @@ #ifndef __DW_HDMI__ #define __DW_HDMI__ -#include - +struct drm_connector; +struct drm_display_mode; +struct drm_encoder; struct dw_hdmi; +struct platform_device; /** * DOC: Supported input formats and encodings -- cgit v1.2.3 From 19126bdf962c2819ed9482ed12d6bd9c37b89b87 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 8 Jan 2019 20:29:34 +0100 Subject: drm: remove include of drmP.h from drm_encoder_slave.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No further changes required. Signed-off-by: Sam Ravnborg Acked-by: Noralf Trønnes Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-8-sam@ravnborg.org --- include/drm/drm_encoder_slave.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/drm/drm_encoder_slave.h b/include/drm/drm_encoder_slave.h index 1107b4b1c599..a09864f6d684 100644 --- a/include/drm/drm_encoder_slave.h +++ b/include/drm/drm_encoder_slave.h @@ -27,7 +27,6 @@ #ifndef __DRM_ENCODER_SLAVE_H__ #define __DRM_ENCODER_SLAVE_H__ -#include #include #include -- cgit v1.2.3 From 785cabaae94e23954d0cb93aad1ac1214606be7d Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 8 Jan 2019 20:29:39 +0100 Subject: drm: remove drmP.h from drm_gem_cma_helper.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With all dependencies fixed we can now remove drmP.h from drm_gem_cma_helper.h. It is replaced by the include files required, or forward declarations as appropritate. Signed-off-by: Sam Ravnborg Acked-by: Noralf Trønnes Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190108192939.15255-13-sam@ravnborg.org --- include/drm/drm_gem_cma_helper.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h index 07c504940ba1..947ac95eb24a 100644 --- a/include/drm/drm_gem_cma_helper.h +++ b/include/drm/drm_gem_cma_helper.h @@ -2,9 +2,12 @@ #ifndef __DRM_GEM_CMA_HELPER_H__ #define __DRM_GEM_CMA_HELPER_H__ -#include +#include +#include #include +struct drm_mode_create_dumb; + /** * struct drm_gem_cma_object - GEM object backed by CMA memory allocations * @base: base GEM object -- cgit v1.2.3 From 13d0add333afea7b2fef77473232b10dea3627dd Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Tue, 8 Jan 2019 19:28:25 +0200 Subject: drm/edid: Pass connector to AVI infoframe functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make life easier for drivers by simply passing the connector to drm_hdmi_avi_infoframe_from_display_mode() and drm_hdmi_avi_infoframe_quant_range(). That way drivers don't need to worry about is_hdmi2_sink mess. v2: Make is_hdmi2_sink() return true for sil-sii8620 Adapt to omap/vc4 changes Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: Archit Taneja Cc: Andrzej Hajda Cc: Laurent Pinchart Cc: Inki Dae Cc: Joonyoung Shim Cc: Seung-Woo Kim Cc: Kyungmin Park Cc: Russell King Cc: CK Hu Cc: Philipp Zabel Cc: Rob Clark Cc: Ben Skeggs Cc: Tomi Valkeinen Cc: Sandy Huang Cc: "Heiko Stübner" Cc: Benjamin Gaignard Cc: Vincent Abriou Cc: Thierry Reding Cc: Eric Anholt Cc: Shawn Guo Cc: Ilia Mirkin Cc: amd-gfx@lists.freedesktop.org Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: nouveau@lists.freedesktop.org Cc: linux-tegra@vger.kernel.org Signed-off-by: Ville Syrjälä Acked-by: Thierry Reding Acked-by: Russell King Reviewed-by: Laurent Pinchart Reviewed-by: Jani Nikula Link: https://patchwork.freedesktop.org/patch/msgid/20190108172828.15184-1-ville.syrjala@linux.intel.com --- drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 3 ++- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 5 +++-- drivers/gpu/drm/bridge/sii902x.c | 3 ++- drivers/gpu/drm/bridge/sil-sii8620.c | 3 +-- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 ++- drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++----------- drivers/gpu/drm/exynos/exynos_hdmi.c | 3 ++- drivers/gpu/drm/i2c/tda998x_drv.c | 3 ++- drivers/gpu/drm/i915/intel_hdmi.c | 14 ++++++------- drivers/gpu/drm/i915/intel_lspcon.c | 15 +++++++------- drivers/gpu/drm/i915/intel_sdvo.c | 10 ++++++---- drivers/gpu/drm/mediatek/mtk_hdmi.c | 3 ++- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 3 ++- drivers/gpu/drm/nouveau/dispnv50/disp.c | 7 +++---- drivers/gpu/drm/omapdrm/omap_encoder.c | 4 ++-- drivers/gpu/drm/radeon/radeon_audio.c | 2 +- drivers/gpu/drm/rockchip/inno_hdmi.c | 4 +++- drivers/gpu/drm/sti/sti_hdmi.c | 3 ++- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 3 ++- drivers/gpu/drm/tegra/hdmi.c | 3 ++- drivers/gpu/drm/tegra/sor.c | 3 ++- drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +++++---- drivers/gpu/drm/zte/zx_hdmi.c | 4 +++- include/drm/drm_edid.h | 8 ++++---- 27 files changed, 91 insertions(+), 66 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index 4cfecdce29a3..1f0426d2fc2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -1682,7 +1682,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder, dce_v10_0_audio_write_sad_regs(encoder); dce_v10_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 7c868916d90f..2280b971d758 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -1724,7 +1724,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder, dce_v11_0_audio_write_sad_regs(encoder); dce_v11_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index 17eaaba36017..db443ec53d3a 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -1423,6 +1423,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder, struct amdgpu_device *adev = dev->dev_private; struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder); struct amdgpu_encoder_atom_dig *dig = amdgpu_encoder->enc_priv; + struct drm_connector *connector = amdgpu_get_connector_for_encoder(encoder); struct hdmi_avi_infoframe frame; u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE]; uint8_t *payload = buffer + 3; @@ -1430,7 +1431,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder, ssize_t err; u32 tmp; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index 8c0576978d36..13da915991dd 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -1616,7 +1616,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder, dce_v8_0_audio_write_sad_regs(encoder); dce_v8_0_audio_write_latency_fields(encoder, mode); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c index f8433c93f463..e11309e9bc4f 100644 --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c @@ -1094,8 +1094,9 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge, mutex_lock(&anx78xx->lock); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode, - false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, + &anx78xx->connector, + adjusted_mode); if (err) { DRM_ERROR("Failed to setup AVI infoframe: %d\n", err); goto unlock; diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index bfa902013aa4..a9b4f45ae87c 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -258,7 +258,8 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge, if (ret) return; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, + &sii902x->connector, adj); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index a6e8f4591e63..0cc293a6ac24 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx, int ret; ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, - mode, - true); + NULL, mode); if (ctx->use_packed_pixel) frm.avi.colorspace = HDMI_COLORSPACE_YUV422; diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 64c3cf027518..88b720b63126 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode) u8 val; /* Initialise info frame from DRM mode */ - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + drm_hdmi_avi_infoframe_from_display_mode(&frame, + &hdmi->connector, mode); if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format)) frame.colorspace = HDMI_COLORSPACE_YUV444; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b506e3622b08..cd25bd08bf53 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector, } EXPORT_SYMBOL(drm_set_preferred_mode); +static bool is_hdmi2_sink(struct drm_connector *connector) +{ + /* + * FIXME: sil-sii8620 doesn't have a connector around when + * we need one, so we have to be prepared for a NULL connector. + */ + if (!connector) + return true; + + return connector->display_info.hdmi.scdc.supported || + connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420; +} + /** * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with * data from a DRM display mode * @frame: HDMI AVI infoframe + * @connector: the connector * @mode: DRM display mode - * @is_hdmi2_sink: Sink is HDMI 2.0 compliant * * Return: 0 on success or a negative error code on failure. */ int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, - const struct drm_display_mode *mode, - bool is_hdmi2_sink) + struct drm_connector *connector, + const struct drm_display_mode *mode) { enum hdmi_picture_aspect picture_aspect; int err; @@ -4864,7 +4877,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we * have to make sure we dont break HDMI 1.4 sinks. */ - if (!is_hdmi2_sink && frame->video_code > 64) + if (!is_hdmi2_sink(connector) && frame->video_code > 64) frame->video_code = 0; /* @@ -4923,21 +4936,17 @@ EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe * quantization range information * @frame: HDMI AVI infoframe + * @connector: the connector * @mode: DRM display mode * @rgb_quant_range: RGB quantization range (Q) * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS) - * @is_hdmi2_sink: HDMI 2.0 sink, which has different default recommendations - * - * Note that @is_hdmi2_sink can be derived by looking at the - * &drm_scdc.supported flag stored in &drm_hdmi_info.scdc, - * &drm_display_info.hdmi, which can be found in &drm_connector.display_info. */ void drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, + struct drm_connector *connector, const struct drm_display_mode *mode, enum hdmi_quantization_range rgb_quant_range, - bool rgb_quant_range_selectable, - bool is_hdmi2_sink) + bool rgb_quant_range_selectable) { /* * CEA-861: @@ -4968,7 +4977,7 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, * we limit non-zero YQ to HDMI 2.0 sinks only as HDMI 2.0 is based * on on CEA-861-F. */ - if (!is_hdmi2_sink || + if (!is_hdmi2_sink(connector) || rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED) frame->ycc_quantization_range = HDMI_YCC_QUANTIZATION_RANGE_LIMITED; diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c index 2092a650df7d..b857df67aff0 100644 --- a/drivers/gpu/drm/exynos/exynos_hdmi.c +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c @@ -819,7 +819,8 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata) return; } - ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, m, false); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, + &hdata->connector, m); if (!ret) ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf)); if (ret > 0) { diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index a7c39f39793f..38c66fbc8276 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode) { union hdmi_infoframe frame; - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, + &priv->connector, mode); frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL; tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index e2c6a2b3e8f2..8a260bd6954c 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -465,15 +465,12 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder, struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; - struct drm_connector *connector = &intel_hdmi->attached_connector->base; - bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported || - connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420; union hdmi_infoframe frame; int ret; ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, - adjusted_mode, - is_hdmi2_sink); + conn_state->connector, + adjusted_mode); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; @@ -486,12 +483,13 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder, else frame.avi.colorspace = HDMI_COLORSPACE_RGB; - drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode, + drm_hdmi_avi_infoframe_quant_range(&frame.avi, + conn_state->connector, + adjusted_mode, crtc_state->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : HDMI_QUANTIZATION_RANGE_FULL, - intel_hdmi->rgb_quant_range_selectable, - is_hdmi2_sink); + intel_hdmi->rgb_quant_range_selectable); drm_hdmi_avi_infoframe_content_type(&frame.avi, conn_state); diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c index 96a8d9524b0c..c18f14d41f1d 100644 --- a/drivers/gpu/drm/i915/intel_lspcon.c +++ b/drivers/gpu/drm/i915/intel_lspcon.c @@ -462,10 +462,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder, uint8_t buf[VIDEO_DIP_DATA_SIZE]; struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base); struct intel_lspcon *lspcon = &dig_port->lspcon; - struct intel_dp *intel_dp = &dig_port->dp; - struct drm_connector *connector = &intel_dp->attached_connector->base; - const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode; - bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported; + const struct drm_display_mode *adjusted_mode = + &crtc_state->base.adjusted_mode; if (!lspcon->active) { DRM_ERROR("Writing infoframes while LSPCON disabled ?\n"); @@ -473,7 +471,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder, } ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, - mode, is_hdmi2_sink); + conn_state->connector, + adjusted_mode); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; @@ -488,11 +487,13 @@ void lspcon_set_infoframes(struct intel_encoder *encoder, frame.avi.colorspace = HDMI_COLORSPACE_RGB; } - drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode, + drm_hdmi_avi_infoframe_quant_range(&frame.avi, + conn_state->connector, + adjusted_mode, crtc_state->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : HDMI_QUANTIZATION_RANGE_FULL, - false, is_hdmi2_sink); + false); ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf)); if (ret < 0) { diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 5805ec1aba12..1277d31adb54 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -981,7 +981,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo, } static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, - const struct intel_crtc_state *pipe_config) + const struct intel_crtc_state *pipe_config, + const struct drm_connector_state *conn_state) { uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)]; union hdmi_infoframe frame; @@ -989,8 +990,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, ssize_t len; ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, - &pipe_config->base.adjusted_mode, - false); + conn_state->connector, + &pipe_config->base.adjusted_mode); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return false; @@ -1316,7 +1317,8 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder, intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI); intel_sdvo_set_colorimetry(intel_sdvo, SDVO_COLORIMETRY_RGB256); - intel_sdvo_set_avi_infoframe(intel_sdvo, crtc_state); + intel_sdvo_set_avi_infoframe(intel_sdvo, + crtc_state, conn_state); } else intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI); diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 862f3ec22131..f3ef7bf80563 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -981,7 +981,8 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi, u8 buffer[17]; ssize_t err; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, + &hdmi->conn, mode); if (err < 0) { dev_err(hdmi->dev, "Failed to get AVI infoframe from mode: %zd\n", err); diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index 7e357077ed26..5ed4cab2819f 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -101,7 +101,8 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi) u32 val; int len; - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, + hdmi->connector, mode); len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer)); if (len < 0) { diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c index 4a56841958c8..f36146d4d9b7 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -554,7 +554,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode) u32 max_ac_packet; union hdmi_infoframe avi_frame; union hdmi_infoframe vendor_frame; - bool scdc_supported, high_tmds_clock_ratio = false, scrambling = false; + bool high_tmds_clock_ratio = false, scrambling = false; u8 config; int ret; int size; @@ -564,10 +564,9 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode) return; hdmi = &nv_connector->base.display_info.hdmi; - scdc_supported = hdmi->scdc.supported; - ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode, - scdc_supported); + ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, + &nv_connector->base, mode); if (!ret) { /* We have an AVI InfoFrame, populate it to the display */ args.pwr.avi_infoframe_length diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c index 452e625f6ce3..4566e0a75cb8 100644 --- a/drivers/gpu/drm/omapdrm/omap_encoder.c +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c @@ -128,8 +128,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder, if (hdmi_mode && dssdev->ops->hdmi.set_infoframe) { struct hdmi_avi_infoframe avi; - r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode, - false); + r = drm_hdmi_avi_infoframe_from_display_mode(&avi, connector, + adjusted_mode); if (r == 0) dssdev->ops->hdmi.set_infoframe(dssdev, &avi); } diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c index 770e31f5fd1b..5a7d48339b32 100644 --- a/drivers/gpu/drm/radeon/radeon_audio.c +++ b/drivers/gpu/drm/radeon/radeon_audio.c @@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder, if (!connector) return -EINVAL; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode); if (err < 0) { DRM_ERROR("failed to setup AVI infoframe: %d\n", err); return err; diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c index 1c02b3e61299..27c945e030a0 100644 --- a/drivers/gpu/drm/rockchip/inno_hdmi.c +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c @@ -295,7 +295,9 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi, union hdmi_infoframe frame; int rc; - rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); + rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, + &hdmi->connector, + mode); if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444) frame.avi.colorspace = HDMI_COLORSPACE_YUV444; diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index ccf718404a1c..4b86878f8ddf 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -434,7 +434,8 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi) DRM_DEBUG_DRIVER("\n"); - ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false); + ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, + hdmi->drm_connector, mode); if (ret < 0) { DRM_ERROR("failed to setup AVI infoframe: %d\n", ret); return ret; diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c index 061d2e0d9011..554a6f4561f3 100644 --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -52,7 +52,8 @@ static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi, u8 buffer[17]; int i, ret; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, + &hdmi->connector, mode); if (ret < 0) { DRM_ERROR("Failed to get infoframes from mode\n"); return ret; diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index 0082468f703c..a7566c67bfb0 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -741,7 +741,8 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi, u8 buffer[17]; ssize_t err; - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, + &hdmi->output.connector, mode); if (err < 0) { dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err); return; diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index b129da2e5afd..d90bf5f6a67a 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -2116,7 +2116,8 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor, value &= ~INFOFRAME_CTRL_ENABLE; tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL); - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false); + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, + &sor->output.connector, mode); if (err < 0) { dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err); return err; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 2f276222e30f..4f9a80f7f242 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -424,18 +424,19 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder) union hdmi_infoframe frame; int ret; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, + hdmi->connector, mode); if (ret < 0) { DRM_ERROR("couldn't fill AVI infoframe\n"); return; } - drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode, + drm_hdmi_avi_infoframe_quant_range(&frame.avi, + hdmi->connector, mode, vc4_encoder->limited_rgb_range ? HDMI_QUANTIZATION_RANGE_LIMITED : HDMI_QUANTIZATION_RANGE_FULL, - vc4_encoder->rgb_range_selectable, - false); + vc4_encoder->rgb_range_selectable); frame.avi.right_bar = cstate->tv.margins.right; frame.avi.left_bar = cstate->tv.margins.left; diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c index 78655269d843..9fc98bb4f3d9 100644 --- a/drivers/gpu/drm/zte/zx_hdmi.c +++ b/drivers/gpu/drm/zte/zx_hdmi.c @@ -125,7 +125,9 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi, union hdmi_infoframe frame; int ret; - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false); + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, + &hdmi->connector, + mode); if (ret) { DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n", ret); diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index e3c404833115..9db6f130df65 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -352,18 +352,18 @@ drm_load_edid_firmware(struct drm_connector *connector) int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, - const struct drm_display_mode *mode, - bool is_hdmi2_sink); + struct drm_connector *connector, + const struct drm_display_mode *mode); int drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, struct drm_connector *connector, const struct drm_display_mode *mode); void drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, + struct drm_connector *connector, const struct drm_display_mode *mode, enum hdmi_quantization_range rgb_quant_range, - bool rgb_quant_range_selectable, - bool is_hdmi2_sink); + bool rgb_quant_range_selectable); /** * drm_eld_mnl - Get ELD monitor name length in bytes. -- cgit v1.2.3 From 1581b2df4cbf614315225f121636e1eb7c80cbae Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Tue, 8 Jan 2019 19:28:28 +0200 Subject: drm/edid: Add display_info.rgb_quant_range_selectable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the CEA-861 QS bit handling entirely into the edid code. No need to bother the drivers with this. Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: amd-gfx@lists.freedesktop.org Cc: Eric Anholt (supporter:DRM DRIVERS FOR VC4) Signed-off-by: Ville Syrjälä Acked-by: Alex Deucher Acked-by: Eric Anholt Link: https://patchwork.freedesktop.org/patch/msgid/20190108172828.15184-4-ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_edid.c | 70 ++++++++++++++++------------------- drivers/gpu/drm/i915/intel_drv.h | 1 - drivers/gpu/drm/i915/intel_hdmi.c | 8 +--- drivers/gpu/drm/i915/intel_lspcon.c | 3 +- drivers/gpu/drm/i915/intel_sdvo.c | 7 +--- drivers/gpu/drm/radeon/radeon_audio.c | 3 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 9 +---- include/drm/drm_connector.h | 6 +++ include/drm/drm_edid.h | 4 +- 9 files changed, 43 insertions(+), 68 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index cd25bd08bf53..990b1909f9d7 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3641,6 +3641,20 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db) return oui == HDMI_FORUM_IEEE_OUI; } +static bool cea_db_is_vcdb(const u8 *db) +{ + if (cea_db_tag(db) != USE_EXTENDED_TAG) + return false; + + if (cea_db_payload_len(db) != 2) + return false; + + if (cea_db_extended_tag(db) != EXT_VIDEO_CAPABILITY_BLOCK) + return false; + + return true; +} + static bool cea_db_is_y420cmdb(const u8 *db) { if (cea_db_tag(db) != USE_EXTENDED_TAG) @@ -4223,41 +4237,6 @@ end: } EXPORT_SYMBOL(drm_detect_monitor_audio); -/** - * drm_rgb_quant_range_selectable - is RGB quantization range selectable? - * @edid: EDID block to scan - * - * Check whether the monitor reports the RGB quantization range selection - * as supported. The AVI infoframe can then be used to inform the monitor - * which quantization range (full or limited) is used. - * - * Return: True if the RGB quantization range is selectable, false otherwise. - */ -bool drm_rgb_quant_range_selectable(struct edid *edid) -{ - u8 *edid_ext; - int i, start, end; - - edid_ext = drm_find_cea_extension(edid); - if (!edid_ext) - return false; - - if (cea_db_offsets(edid_ext, &start, &end)) - return false; - - for_each_cea_db(edid_ext, i, start, end) { - if (cea_db_tag(&edid_ext[i]) == USE_EXTENDED_TAG && - cea_db_payload_len(&edid_ext[i]) == 2 && - cea_db_extended_tag(&edid_ext[i]) == - EXT_VIDEO_CAPABILITY_BLOCK) { - DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]); - return edid_ext[i + 2] & EDID_CEA_VCDB_QS; - } - } - - return false; -} -EXPORT_SYMBOL(drm_rgb_quant_range_selectable); /** * drm_default_rgb_quant_range - default RGB quantization range @@ -4278,6 +4257,16 @@ drm_default_rgb_quant_range(const struct drm_display_mode *mode) } EXPORT_SYMBOL(drm_default_rgb_quant_range); +static void drm_parse_vcdb(struct drm_connector *connector, const u8 *db) +{ + struct drm_display_info *info = &connector->display_info; + + DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", db[2]); + + if (db[2] & EDID_CEA_VCDB_QS) + info->rgb_quant_range_selectable = true; +} + static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector, const u8 *db) { @@ -4452,6 +4441,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector, drm_parse_hdmi_forum_vsdb(connector, db); if (cea_db_is_y420cmdb(db)) drm_parse_y420cmdb_bitmap(connector, db); + if (cea_db_is_vcdb(db)) + drm_parse_vcdb(connector, db); } } @@ -4472,6 +4463,7 @@ drm_reset_display_info(struct drm_connector *connector) info->max_tmds_clock = 0; info->dvi_dual = false; info->has_hdmi_infoframe = false; + info->rgb_quant_range_selectable = false; memset(&info->hdmi, 0, sizeof(info->hdmi)); info->non_desktop = 0; @@ -4939,15 +4931,15 @@ EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode); * @connector: the connector * @mode: DRM display mode * @rgb_quant_range: RGB quantization range (Q) - * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS) */ void drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, struct drm_connector *connector, const struct drm_display_mode *mode, - enum hdmi_quantization_range rgb_quant_range, - bool rgb_quant_range_selectable) + enum hdmi_quantization_range rgb_quant_range) { + const struct drm_display_info *info = &connector->display_info; + /* * CEA-861: * "A Source shall not send a non-zero Q value that does not correspond @@ -4958,7 +4950,7 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, * HDMI 2.0 recommends sending non-zero Q when it does match the * default RGB quantization range for the mode, even when QS=0. */ - if (rgb_quant_range_selectable || + if (info->rgb_quant_range_selectable || rgb_quant_range == drm_default_rgb_quant_range(mode)) frame->quantization_range = rgb_quant_range; else diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a7d9ac912125..153ee47d9c03 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1057,7 +1057,6 @@ struct intel_hdmi { } dp_dual_mode; bool has_hdmi_sink; bool has_audio; - bool rgb_quant_range_selectable; struct intel_connector *attached_connector; struct cec_notifier *cec_notifier; }; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 8a260bd6954c..4d264e577e3b 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -462,7 +462,6 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder, const struct intel_crtc_state *crtc_state, const struct drm_connector_state *conn_state) { - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); const struct drm_display_mode *adjusted_mode = &crtc_state->base.adjusted_mode; union hdmi_infoframe frame; @@ -488,8 +487,7 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder, adjusted_mode, crtc_state->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : - HDMI_QUANTIZATION_RANGE_FULL, - intel_hdmi->rgb_quant_range_selectable); + HDMI_QUANTIZATION_RANGE_FULL); drm_hdmi_avi_infoframe_content_type(&frame.avi, conn_state); @@ -1816,7 +1814,6 @@ intel_hdmi_unset_edid(struct drm_connector *connector) intel_hdmi->has_hdmi_sink = false; intel_hdmi->has_audio = false; - intel_hdmi->rgb_quant_range_selectable = false; intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE; intel_hdmi->dp_dual_mode.max_tmds_clock = 0; @@ -1900,9 +1897,6 @@ intel_hdmi_set_edid(struct drm_connector *connector) to_intel_connector(connector)->detect_edid = edid; if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { - intel_hdmi->rgb_quant_range_selectable = - drm_rgb_quant_range_selectable(edid); - intel_hdmi->has_audio = drm_detect_monitor_audio(edid); intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c index c18f14d41f1d..7d15be5932e0 100644 --- a/drivers/gpu/drm/i915/intel_lspcon.c +++ b/drivers/gpu/drm/i915/intel_lspcon.c @@ -492,8 +492,7 @@ void lspcon_set_infoframes(struct intel_encoder *encoder, adjusted_mode, crtc_state->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : - HDMI_QUANTIZATION_RANGE_FULL, - false); + HDMI_QUANTIZATION_RANGE_FULL); ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf)); if (ret < 0) { diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 9c16e273fb8d..669fa9faad70 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -103,7 +103,6 @@ struct intel_sdvo { bool has_hdmi_monitor; bool has_hdmi_audio; - bool rgb_quant_range_selectable; /* DDC bus used by this SDVO encoder */ uint8_t ddc_bus; @@ -1004,8 +1003,7 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo, adjusted_mode, pipe_config->limited_color_range ? HDMI_QUANTIZATION_RANGE_LIMITED : - HDMI_QUANTIZATION_RANGE_FULL, - intel_sdvo->rgb_quant_range_selectable); + HDMI_QUANTIZATION_RANGE_FULL); len = hdmi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data)); if (len < 0) @@ -1805,8 +1803,6 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector) if (intel_sdvo_connector->is_hdmi) { intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid); intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid); - intel_sdvo->rgb_quant_range_selectable = - drm_rgb_quant_range_selectable(edid); } } else status = connector_status_disconnected; @@ -1855,7 +1851,6 @@ intel_sdvo_detect(struct drm_connector *connector, bool force) intel_sdvo->has_hdmi_monitor = false; intel_sdvo->has_hdmi_audio = false; - intel_sdvo->rgb_quant_range_selectable = false; if ((intel_sdvo_connector->output_flag & response) == 0) ret = connector_status_disconnected; diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c index 708765bf9e66..96f71114237a 100644 --- a/drivers/gpu/drm/radeon/radeon_audio.c +++ b/drivers/gpu/drm/radeon/radeon_audio.c @@ -526,8 +526,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder, drm_hdmi_avi_infoframe_quant_range(&frame, connector, mode, radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB ? HDMI_QUANTIZATION_RANGE_LIMITED : - HDMI_QUANTIZATION_RANGE_FULL, - drm_rgb_quant_range_selectable(radeon_connector_edid(connector))); + HDMI_QUANTIZATION_RANGE_FULL); } err = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer)); diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 4f9a80f7f242..051b61b62541 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -109,7 +109,6 @@ struct vc4_hdmi_encoder { struct vc4_encoder base; bool hdmi_monitor; bool limited_rgb_range; - bool rgb_range_selectable; }; static inline struct vc4_hdmi_encoder * @@ -280,11 +279,6 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid); - if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { - vc4_encoder->rgb_range_selectable = - drm_rgb_quant_range_selectable(edid); - } - drm_connector_update_edid_property(connector, edid); ret = drm_add_edid_modes(connector, edid); kfree(edid); @@ -435,8 +429,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder) hdmi->connector, mode, vc4_encoder->limited_rgb_range ? HDMI_QUANTIZATION_RANGE_LIMITED : - HDMI_QUANTIZATION_RANGE_FULL, - vc4_encoder->rgb_range_selectable); + HDMI_QUANTIZATION_RANGE_FULL); frame.avi.right_bar = cstate->tv.margins.right; frame.avi.left_bar = cstate->tv.margins.left; diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 594f8d33a61f..e8ad30d7561a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -365,6 +365,12 @@ struct drm_display_info { */ bool has_hdmi_infoframe; + /** + * @rgb_quant_range_selectable: Does the sink support selecting + * the RGB quantization range? + */ + bool rgb_quant_range_selectable; + /** * @edid_hdmi_dc_modes: Mask of supported hdmi deep color modes. Even * more stuff redundant with @bus_formats. diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index 9db6f130df65..8dc1a081fb36 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -362,8 +362,7 @@ void drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame, struct drm_connector *connector, const struct drm_display_mode *mode, - enum hdmi_quantization_range rgb_quant_range, - bool rgb_quant_range_selectable); + enum hdmi_quantization_range rgb_quant_range); /** * drm_eld_mnl - Get ELD monitor name length in bytes. @@ -471,7 +470,6 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match); enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code); bool drm_detect_hdmi_monitor(struct edid *edid); bool drm_detect_monitor_audio(struct edid *edid); -bool drm_rgb_quant_range_selectable(struct edid *edid); enum hdmi_quantization_range drm_default_rgb_quant_range(const struct drm_display_mode *mode); int drm_add_modes_noedid(struct drm_connector *connector, -- cgit v1.2.3 From ebcc0e6b509108b4a67daa4c55809a05ab7f4b77 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Thu, 10 Jan 2019 19:53:29 -0500 Subject: drm/dp_mst: Introduce new refcounting scheme for mstbs and ports The current way of handling refcounting in the DP MST helpers is really confusing and probably just plain wrong because it's been hacked up many times over the years without anyone actually going over the code and seeing if things could be simplified. To the best of my understanding, the current scheme works like this: drm_dp_mst_port and drm_dp_mst_branch both have a single refcount. When this refcount hits 0 for either of the two, they're removed from the topology state, but not immediately freed. Both ports and branch devices will reinitialize their kref once it's hit 0 before actually destroying themselves. The intended purpose behind this is so that we can avoid problems like not being able to free a remote payload that might still be active, due to us having removed all of the port/branch device structures in memory, as per: commit 91a25e463130 ("drm/dp/mst: deallocate payload on port destruction") Which may have worked, but then it caused use-after-free errors. Being new to MST at the time, I tried fixing it; commit 263efde31f97 ("drm/dp/mst: Get validated port ref in drm_dp_update_payload_part1()") But, that was broken: both drm_dp_mst_port and drm_dp_mst_branch structs are validated in almost every DP MST helper function. Simply put, this means we go through the topology and try to see if the given drm_dp_mst_branch or drm_dp_mst_port is still attached to something before trying to use it in order to avoid dereferencing freed memory (something that has happened a LOT in the past with this library). Because of this it doesn't actually matter whether or not we keep keep the ports and branches around in memory as that's not enough, because any function that validates the branches and ports passed to it will still reject them anyway since they're no longer in the topology structure. So, use-after-free errors were fixed but payload deallocation was completely broken. Two years later, AMD informed me about this issue and I attempted to come up with a temporary fix, pending a long-overdue cleanup of this library: commit c54c7374ff44 ("drm/dp_mst: Skip validating ports during destruction, just ref") But then that introduced use-after-free errors, so I quickly reverted it: commit 9765635b3075 ("Revert "drm/dp_mst: Skip validating ports during destruction, just ref"") And in the process, learned that there is just no simple fix for this: the design is just broken. Unfortunately, the usage of these helpers are quite broken as well. Some drivers like i915 have been smart enough to avoid accessing any kind of information from MST port structures, but others like nouveau have assumed, understandably so, that drm_dp_mst_port structures are normal and can just be accessed at any time without worrying about use-after-free errors. After a lot of discussion, me and Daniel Vetter came up with a better idea to replace all of this. To summarize, since this is documented far more indepth in the documentation this patch introduces, we make it so that drm_dp_mst_port and drm_dp_mst_branch structures have two different classes of refcounts: topology_kref, and malloc_kref. topology_kref corresponds to the lifetime of the given drm_dp_mst_port or drm_dp_mst_branch in it's given topology. Once it hits zero, any associated connectors are removed and the branch or port can no longer be validated. malloc_kref corresponds to the lifetime of the memory allocation for the actual structure, and will always be non-zero so long as the topology_kref is non-zero. This gives us a way to allow callers to hold onto port and branch device structures past their topology lifetime, and dramatically simplifies the lifetimes of both structures. This also finally fixes the port deallocation problem, properly. Additionally: since this now means that we can keep ports and branch devices allocated in memory for however long we need, we no longer need a significant amount of the port validation that we currently do. Additionally, there is one last scenario that this fixes, which couldn't have been fixed properly beforehand: - CPU1 unrefs port from topology (refcount 1->0) - CPU2 refs port in topology(refcount 0->1) Since we now can guarantee memory safety for ports and branches as-needed, we also can make our main reference counting functions fix this problem by using kref_get_unless_zero() internally so that topology refcounts can only ever reach 0 once. Changes since v4: * Change the kernel-figure summary for dp-mst/topology-figure-1.dot a bit - danvet * Remove figure numbers - danvet Changes since v3: * Remove rebase detritus - danvet * Split out purely style changes into separate patches - hwentlan Changes since v2: * Fix commit message - checkpatch * s/)-1/) - 1/g - checkpatch Changes since v1: * Remove forward declarations - danvet * Move "Branch device and port refcounting" section from documentation into kernel-doc comments - danvet * Export internal topology lifetime functions into their own section in the kernel-docs - danvet * s/@/&/g for struct references in kernel-docs - danvet * Drop the "when they are no longer being used" bits from the kernel docs - danvet * Modify diagrams to show how the DRM driver interacts with the topology and payloads - danvet * Make suggested documentation changes for drm_dp_mst_topology_get_mstb() and drm_dp_mst_topology_get_port() - danvet * Better explain the relationship between malloc refs and topology krefs in the documentation for drm_dp_mst_topology_get_port() and drm_dp_mst_topology_get_mstb() - danvet * Fix "See also" in drm_dp_mst_topology_get_mstb() - danvet * Rename drm_dp_mst_topology_get_(port|mstb)() -> drm_dp_mst_topology_try_get_(port|mstb)() and drm_dp_mst_topology_ref_(port|mstb)() -> drm_dp_mst_topology_get_(port|mstb)() - danvet * s/should/must in docs - danvet * WARN_ON(refcount == 0) in topology_get_(mstb|port) - danvet * Move kdocs for mstb/port structs inline - danvet * Split drm_dp_get_last_connected_port_and_mstb() changes into their own commit - danvet Signed-off-by: Lyude Paul Reviewed-by: Harry Wentland Reviewed-by: Daniel Vetter Cc: David Airlie Cc: Jerry Zuo Cc: Juston Li Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-7-lyude@redhat.com --- Documentation/gpu/dp-mst/topology-figure-1.dot | 52 +++ Documentation/gpu/dp-mst/topology-figure-2.dot | 56 +++ Documentation/gpu/dp-mst/topology-figure-3.dot | 59 ++++ Documentation/gpu/drm-kms-helpers.rst | 26 +- drivers/gpu/drm/drm_dp_mst_topology.c | 458 +++++++++++++++++++++---- include/drm/drm_dp_mst_helper.h | 32 +- 6 files changed, 614 insertions(+), 69 deletions(-) create mode 100644 Documentation/gpu/dp-mst/topology-figure-1.dot create mode 100644 Documentation/gpu/dp-mst/topology-figure-2.dot create mode 100644 Documentation/gpu/dp-mst/topology-figure-3.dot (limited to 'include') diff --git a/Documentation/gpu/dp-mst/topology-figure-1.dot b/Documentation/gpu/dp-mst/topology-figure-1.dot new file mode 100644 index 000000000000..157e17c7e0b0 --- /dev/null +++ b/Documentation/gpu/dp-mst/topology-figure-1.dot @@ -0,0 +1,52 @@ +digraph T { + /* Make sure our payloads are always drawn below the driver node */ + subgraph cluster_driver { + fillcolor = grey; + style = filled; + driver -> {payload1, payload2} [dir=none]; + } + + /* Driver malloc references */ + edge [style=dashed]; + driver -> port1; + driver -> port2; + driver -> port3:e; + driver -> port4; + + payload1:s -> port1:e; + payload2:s -> port3:e; + edge [style=""]; + + subgraph cluster_topology { + label="Topology Manager"; + labelloc=bottom; + + /* Topology references */ + mstb1 -> {port1, port2}; + port1 -> mstb2; + port2 -> mstb3 -> {port3, port4}; + port3 -> mstb4; + + /* Malloc references */ + edge [style=dashed;dir=back]; + mstb1 -> {port1, port2}; + port1 -> mstb2; + port2 -> mstb3 -> {port3, port4}; + port3 -> mstb4; + } + + driver [label="DRM driver";style=filled;shape=box;fillcolor=lightblue]; + + payload1 [label="Payload #1";style=filled;shape=box;fillcolor=lightblue]; + payload2 [label="Payload #2";style=filled;shape=box;fillcolor=lightblue]; + + mstb1 [label="MSTB #1";style=filled;fillcolor=palegreen;shape=oval]; + mstb2 [label="MSTB #2";style=filled;fillcolor=palegreen;shape=oval]; + mstb3 [label="MSTB #3";style=filled;fillcolor=palegreen;shape=oval]; + mstb4 [label="MSTB #4";style=filled;fillcolor=palegreen;shape=oval]; + + port1 [label="Port #1";shape=oval]; + port2 [label="Port #2";shape=oval]; + port3 [label="Port #3";shape=oval]; + port4 [label="Port #4";shape=oval]; +} diff --git a/Documentation/gpu/dp-mst/topology-figure-2.dot b/Documentation/gpu/dp-mst/topology-figure-2.dot new file mode 100644 index 000000000000..4243dd1737cb --- /dev/null +++ b/Documentation/gpu/dp-mst/topology-figure-2.dot @@ -0,0 +1,56 @@ +digraph T { + /* Make sure our payloads are always drawn below the driver node */ + subgraph cluster_driver { + fillcolor = grey; + style = filled; + driver -> {payload1, payload2} [dir=none]; + } + + /* Driver malloc references */ + edge [style=dashed]; + driver -> port1; + driver -> port2; + driver -> port3:e; + driver -> port4 [color=red]; + + payload1:s -> port1:e; + payload2:s -> port3:e; + edge [style=""]; + + subgraph cluster_topology { + label="Topology Manager"; + labelloc=bottom; + + /* Topology references */ + mstb1 -> {port1, port2}; + port1 -> mstb2; + edge [color=red]; + port2 -> mstb3 -> {port3, port4}; + port3 -> mstb4; + edge [color=""]; + + /* Malloc references */ + edge [style=dashed;dir=back]; + mstb1 -> {port1, port2}; + port1 -> mstb2; + port2 -> mstb3 -> port3; + edge [color=red]; + mstb3 -> port4; + port3 -> mstb4; + } + + mstb1 [label="MSTB #1";style=filled;fillcolor=palegreen]; + mstb2 [label="MSTB #2";style=filled;fillcolor=palegreen]; + mstb3 [label="MSTB #3";style=filled;fillcolor=palegreen]; + mstb4 [label="MSTB #4";style=filled;fillcolor=grey]; + + port1 [label="Port #1"]; + port2 [label="Port #2"]; + port3 [label="Port #3"]; + port4 [label="Port #4";style=filled;fillcolor=grey]; + + driver [label="DRM driver";style=filled;shape=box;fillcolor=lightblue]; + + payload1 [label="Payload #1";style=filled;shape=box;fillcolor=lightblue]; + payload2 [label="Payload #2";style=filled;shape=box;fillcolor=lightblue]; +} diff --git a/Documentation/gpu/dp-mst/topology-figure-3.dot b/Documentation/gpu/dp-mst/topology-figure-3.dot new file mode 100644 index 000000000000..6cd78d06778b --- /dev/null +++ b/Documentation/gpu/dp-mst/topology-figure-3.dot @@ -0,0 +1,59 @@ +digraph T { + /* Make sure our payloads are always drawn below the driver node */ + subgraph cluster_driver { + fillcolor = grey; + style = filled; + edge [dir=none]; + driver -> payload1; + driver -> payload2 [penwidth=3]; + edge [dir=""]; + } + + /* Driver malloc references */ + edge [style=dashed]; + driver -> port1; + driver -> port2; + driver -> port3:e; + driver -> port4 [color=grey]; + payload1:s -> port1:e; + payload2:s -> port3:e [penwidth=3]; + edge [style=""]; + + subgraph cluster_topology { + label="Topology Manager"; + labelloc=bottom; + + /* Topology references */ + mstb1 -> {port1, port2}; + port1 -> mstb2; + edge [color=grey]; + port2 -> mstb3 -> {port3, port4}; + port3 -> mstb4; + edge [color=""]; + + /* Malloc references */ + edge [style=dashed;dir=back]; + mstb1 -> {port1, port2}; + port1 -> mstb2; + port2 -> mstb3 [penwidth=3]; + mstb3 -> port3 [penwidth=3]; + edge [color=grey]; + mstb3 -> port4; + port3 -> mstb4; + } + + mstb1 [label="MSTB #1";style=filled;fillcolor=palegreen]; + mstb2 [label="MSTB #2";style=filled;fillcolor=palegreen]; + mstb3 [label="MSTB #3";style=filled;fillcolor=palegreen;penwidth=3]; + mstb4 [label="MSTB #4";style=filled;fillcolor=grey]; + + port1 [label="Port #1"]; + port2 [label="Port #2";penwidth=5]; + port3 [label="Port #3";penwidth=3]; + port4 [label="Port #4";style=filled;fillcolor=grey]; + + driver [label="DRM driver";style=filled;shape=box;fillcolor=lightblue]; + + payload1 [label="Payload #1";style=filled;shape=box;fillcolor=lightblue]; + payload2 [label="Payload #2";style=filled;shape=box;fillcolor=lightblue;penwidth=3]; +} diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 4b4dc236ef6f..27001ae5d370 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -208,18 +208,40 @@ Display Port Dual Mode Adaptor Helper Functions Reference .. kernel-doc:: drivers/gpu/drm/drm_dp_dual_mode_helper.c :export: -Display Port MST Helper Functions Reference -=========================================== +Display Port MST Helpers +======================== + +Overview +-------- .. kernel-doc:: drivers/gpu/drm/drm_dp_mst_topology.c :doc: dp mst helper +.. kernel-doc:: drivers/gpu/drm/drm_dp_mst_topology.c + :doc: Branch device and port refcounting + +Functions Reference +------------------- + .. kernel-doc:: include/drm/drm_dp_mst_helper.h :internal: .. kernel-doc:: drivers/gpu/drm/drm_dp_mst_topology.c :export: +Topology Lifetime Internals +--------------------------- + +These functions aren't exported to drivers, but are documented here to help make +the MST topology helpers easier to understand + +.. kernel-doc:: drivers/gpu/drm/drm_dp_mst_topology.c + :functions: drm_dp_mst_topology_try_get_mstb drm_dp_mst_topology_get_mstb + drm_dp_mst_topology_put_mstb + drm_dp_mst_topology_try_get_port drm_dp_mst_topology_get_port + drm_dp_mst_topology_put_port + drm_dp_mst_get_mstb_malloc drm_dp_mst_put_mstb_malloc + MIPI DSI Helper Functions Reference =================================== diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 074e985093ca..796985609933 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -850,46 +850,212 @@ static struct drm_dp_mst_branch *drm_dp_add_mst_branch_device(u8 lct, u8 *rad) if (lct > 1) memcpy(mstb->rad, rad, lct / 2); INIT_LIST_HEAD(&mstb->ports); - kref_init(&mstb->kref); + kref_init(&mstb->topology_kref); + kref_init(&mstb->malloc_kref); return mstb; } -static void drm_dp_free_mst_port(struct kref *kref); - static void drm_dp_free_mst_branch_device(struct kref *kref) { - struct drm_dp_mst_branch *mstb = container_of(kref, struct drm_dp_mst_branch, kref); - if (mstb->port_parent) { - if (list_empty(&mstb->port_parent->next)) - kref_put(&mstb->port_parent->kref, drm_dp_free_mst_port); - } + struct drm_dp_mst_branch *mstb = + container_of(kref, struct drm_dp_mst_branch, malloc_kref); + + if (mstb->port_parent) + drm_dp_mst_put_port_malloc(mstb->port_parent); + kfree(mstb); } +/** + * DOC: Branch device and port refcounting + * + * Topology refcount overview + * ~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * The refcounting schemes for &struct drm_dp_mst_branch and &struct + * drm_dp_mst_port are somewhat unusual. Both ports and branch devices have + * two different kinds of refcounts: topology refcounts, and malloc refcounts. + * + * Topology refcounts are not exposed to drivers, and are handled internally + * by the DP MST helpers. The helpers use them in order to prevent the + * in-memory topology state from being changed in the middle of critical + * operations like changing the internal state of payload allocations. This + * means each branch and port will be considered to be connected to the rest + * of the topology until it's topology refcount reaches zero. Additionally, + * for ports this means that their associated &struct drm_connector will stay + * registered with userspace until the port's refcount reaches 0. + * + * Malloc refcount overview + * ~~~~~~~~~~~~~~~~~~~~~~~~ + * + * Malloc references are used to keep a &struct drm_dp_mst_port or &struct + * drm_dp_mst_branch allocated even after all of its topology references have + * been dropped, so that the driver or MST helpers can safely access each + * branch's last known state before it was disconnected from the topology. + * When the malloc refcount of a port or branch reaches 0, the memory + * allocation containing the &struct drm_dp_mst_branch or &struct + * drm_dp_mst_port respectively will be freed. + * + * For &struct drm_dp_mst_branch, malloc refcounts are not currently exposed + * to drivers. As of writing this documentation, there are no drivers that + * have a usecase for accessing &struct drm_dp_mst_branch outside of the MST + * helpers. Exposing this API to drivers in a race-free manner would take more + * tweaking of the refcounting scheme, however patches are welcome provided + * there is a legitimate driver usecase for this. + * + * Refcount relationships in a topology + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + * + * Let's take a look at why the relationship between topology and malloc + * refcounts is designed the way it is. + * + * .. kernel-figure:: dp-mst/topology-figure-1.dot + * + * An example of topology and malloc refs in a DP MST topology with two + * active payloads. Topology refcount increments are indicated by solid + * lines, and malloc refcount increments are indicated by dashed lines. + * Each starts from the branch which incremented the refcount, and ends at + * the branch to which the refcount belongs to, i.e. the arrow points the + * same way as the C pointers used to reference a structure. + * + * As you can see in the above figure, every branch increments the topology + * refcount of it's children, and increments the malloc refcount of it's + * parent. Additionally, every payload increments the malloc refcount of it's + * assigned port by 1. + * + * So, what would happen if MSTB #3 from the above figure was unplugged from + * the system, but the driver hadn't yet removed payload #2 from port #3? The + * topology would start to look like the figure below. + * + * .. kernel-figure:: dp-mst/topology-figure-2.dot + * + * Ports and branch devices which have been released from memory are + * colored grey, and references which have been removed are colored red. + * + * Whenever a port or branch device's topology refcount reaches zero, it will + * decrement the topology refcounts of all its children, the malloc refcount + * of its parent, and finally its own malloc refcount. For MSTB #4 and port + * #4, this means they both have been disconnected from the topology and freed + * from memory. But, because payload #2 is still holding a reference to port + * #3, port #3 is removed from the topology but it's &struct drm_dp_mst_port + * is still accessible from memory. This also means port #3 has not yet + * decremented the malloc refcount of MSTB #3, so it's &struct + * drm_dp_mst_branch will also stay allocated in memory until port #3's + * malloc refcount reaches 0. + * + * This relationship is necessary because in order to release payload #2, we + * need to be able to figure out the last relative of port #3 that's still + * connected to the topology. In this case, we would travel up the topology as + * shown below. + * + * .. kernel-figure:: dp-mst/topology-figure-3.dot + * + * And finally, remove payload #2 by communicating with port #2 through + * sideband transactions. + */ + +/** + * drm_dp_mst_get_mstb_malloc() - Increment the malloc refcount of a branch + * device + * @mstb: The &struct drm_dp_mst_branch to increment the malloc refcount of + * + * Increments &drm_dp_mst_branch.malloc_kref. When + * &drm_dp_mst_branch.malloc_kref reaches 0, the memory allocation for @mstb + * will be released and @mstb may no longer be used. + * + * See also: drm_dp_mst_put_mstb_malloc() + */ +static void +drm_dp_mst_get_mstb_malloc(struct drm_dp_mst_branch *mstb) +{ + kref_get(&mstb->malloc_kref); + DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->malloc_kref)); +} + +/** + * drm_dp_mst_put_mstb_malloc() - Decrement the malloc refcount of a branch + * device + * @mstb: The &struct drm_dp_mst_branch to decrement the malloc refcount of + * + * Decrements &drm_dp_mst_branch.malloc_kref. When + * &drm_dp_mst_branch.malloc_kref reaches 0, the memory allocation for @mstb + * will be released and @mstb may no longer be used. + * + * See also: drm_dp_mst_get_mstb_malloc() + */ +static void +drm_dp_mst_put_mstb_malloc(struct drm_dp_mst_branch *mstb) +{ + DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->malloc_kref) - 1); + kref_put(&mstb->malloc_kref, drm_dp_free_mst_branch_device); +} + +static void drm_dp_free_mst_port(struct kref *kref) +{ + struct drm_dp_mst_port *port = + container_of(kref, struct drm_dp_mst_port, malloc_kref); + + drm_dp_mst_put_mstb_malloc(port->parent); + kfree(port); +} + +/** + * drm_dp_mst_get_port_malloc() - Increment the malloc refcount of an MST port + * @port: The &struct drm_dp_mst_port to increment the malloc refcount of + * + * Increments &drm_dp_mst_port.malloc_kref. When &drm_dp_mst_port.malloc_kref + * reaches 0, the memory allocation for @port will be released and @port may + * no longer be used. + * + * Because @port could potentially be freed at any time by the DP MST helpers + * if &drm_dp_mst_port.malloc_kref reaches 0, including during a call to this + * function, drivers that which to make use of &struct drm_dp_mst_port should + * ensure that they grab at least one main malloc reference to their MST ports + * in &drm_dp_mst_topology_cbs.add_connector. This callback is called before + * there is any chance for &drm_dp_mst_port.malloc_kref to reach 0. + * + * See also: drm_dp_mst_put_port_malloc() + */ +void +drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port) +{ + kref_get(&port->malloc_kref); + DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->malloc_kref)); +} +EXPORT_SYMBOL(drm_dp_mst_get_port_malloc); + +/** + * drm_dp_mst_put_port_malloc() - Decrement the malloc refcount of an MST port + * @port: The &struct drm_dp_mst_port to decrement the malloc refcount of + * + * Decrements &drm_dp_mst_port.malloc_kref. When &drm_dp_mst_port.malloc_kref + * reaches 0, the memory allocation for @port will be released and @port may + * no longer be used. + * + * See also: drm_dp_mst_get_port_malloc() + */ +void +drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port) +{ + DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->malloc_kref) - 1); + kref_put(&port->malloc_kref, drm_dp_free_mst_port); +} +EXPORT_SYMBOL(drm_dp_mst_put_port_malloc); + static void drm_dp_destroy_mst_branch_device(struct kref *kref) { - struct drm_dp_mst_branch *mstb = container_of(kref, struct drm_dp_mst_branch, kref); + struct drm_dp_mst_branch *mstb = + container_of(kref, struct drm_dp_mst_branch, topology_kref); + struct drm_dp_mst_topology_mgr *mgr = mstb->mgr; struct drm_dp_mst_port *port, *tmp; bool wake_tx = false; - /* - * init kref again to be used by ports to remove mst branch when it is - * not needed anymore - */ - kref_init(kref); - - if (mstb->port_parent && list_empty(&mstb->port_parent->next)) - kref_get(&mstb->port_parent->kref); - - /* - * destroy all ports - don't need lock - * as there are no more references to the mst branch - * device at this point. - */ + mutex_lock(&mgr->lock); list_for_each_entry_safe(port, tmp, &mstb->ports, next) { list_del(&port->next); drm_dp_mst_topology_put_port(port); } + mutex_unlock(&mgr->lock); /* drop any tx slots msg */ mutex_lock(&mstb->mgr->qlock); @@ -908,14 +1074,83 @@ static void drm_dp_destroy_mst_branch_device(struct kref *kref) if (wake_tx) wake_up_all(&mstb->mgr->tx_waitq); - kref_put(kref, drm_dp_free_mst_branch_device); + drm_dp_mst_put_mstb_malloc(mstb); } -static void drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb) +/** + * drm_dp_mst_topology_try_get_mstb() - Increment the topology refcount of a + * branch device unless its zero + * @mstb: &struct drm_dp_mst_branch to increment the topology refcount of + * + * Attempts to grab a topology reference to @mstb, if it hasn't yet been + * removed from the topology (e.g. &drm_dp_mst_branch.topology_kref has + * reached 0). Holding a topology reference implies that a malloc reference + * will be held to @mstb as long as the user holds the topology reference. + * + * Care should be taken to ensure that the user has at least one malloc + * reference to @mstb. If you already have a topology reference to @mstb, you + * should use drm_dp_mst_topology_get_mstb() instead. + * + * See also: + * drm_dp_mst_topology_get_mstb() + * drm_dp_mst_topology_put_mstb() + * + * Returns: + * * 1: A topology reference was grabbed successfully + * * 0: @port is no longer in the topology, no reference was grabbed + */ +static int __must_check +drm_dp_mst_topology_try_get_mstb(struct drm_dp_mst_branch *mstb) { - kref_put(&mstb->kref, drm_dp_destroy_mst_branch_device); + int ret = kref_get_unless_zero(&mstb->topology_kref); + + if (ret) + DRM_DEBUG("mstb %p (%d)\n", mstb, + kref_read(&mstb->topology_kref)); + + return ret; } +/** + * drm_dp_mst_topology_get_mstb() - Increment the topology refcount of a + * branch device + * @mstb: The &struct drm_dp_mst_branch to increment the topology refcount of + * + * Increments &drm_dp_mst_branch.topology_refcount without checking whether or + * not it's already reached 0. This is only valid to use in scenarios where + * you are already guaranteed to have at least one active topology reference + * to @mstb. Otherwise, drm_dp_mst_topology_try_get_mstb() must be used. + * + * See also: + * drm_dp_mst_topology_try_get_mstb() + * drm_dp_mst_topology_put_mstb() + */ +static void drm_dp_mst_topology_get_mstb(struct drm_dp_mst_branch *mstb) +{ + WARN_ON(kref_read(&mstb->topology_kref) == 0); + kref_get(&mstb->topology_kref); + DRM_DEBUG("mstb %p (%d)\n", mstb, kref_read(&mstb->topology_kref)); +} + +/** + * drm_dp_mst_topology_put_mstb() - release a topology reference to a branch + * device + * @mstb: The &struct drm_dp_mst_branch to release the topology reference from + * + * Releases a topology reference from @mstb by decrementing + * &drm_dp_mst_branch.topology_kref. + * + * See also: + * drm_dp_mst_topology_try_get_mstb() + * drm_dp_mst_topology_get_mstb() + */ +static void +drm_dp_mst_topology_put_mstb(struct drm_dp_mst_branch *mstb) +{ + DRM_DEBUG("mstb %p (%d)\n", + mstb, kref_read(&mstb->topology_kref) - 1); + kref_put(&mstb->topology_kref, drm_dp_destroy_mst_branch_device); +} static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) { @@ -937,7 +1172,8 @@ static void drm_dp_port_teardown_pdt(struct drm_dp_mst_port *port, int old_pdt) static void drm_dp_destroy_port(struct kref *kref) { - struct drm_dp_mst_port *port = container_of(kref, struct drm_dp_mst_port, kref); + struct drm_dp_mst_port *port = + container_of(kref, struct drm_dp_mst_port, topology_kref); struct drm_dp_mst_topology_mgr *mgr = port->mgr; if (!port->input) { @@ -956,7 +1192,6 @@ static void drm_dp_destroy_port(struct kref *kref) * from an EDID retrieval */ mutex_lock(&mgr->destroy_connector_lock); - kref_get(&port->parent->kref); list_add(&port->next, &mgr->destroy_connector_list); mutex_unlock(&mgr->destroy_connector_lock); schedule_work(&mgr->destroy_connector_work); @@ -967,12 +1202,79 @@ static void drm_dp_destroy_port(struct kref *kref) drm_dp_port_teardown_pdt(port, port->pdt); port->pdt = DP_PEER_DEVICE_NONE; } - kfree(port); + drm_dp_mst_put_port_malloc(port); +} + +/** + * drm_dp_mst_topology_try_get_port() - Increment the topology refcount of a + * port unless its zero + * @port: &struct drm_dp_mst_port to increment the topology refcount of + * + * Attempts to grab a topology reference to @port, if it hasn't yet been + * removed from the topology (e.g. &drm_dp_mst_port.topology_kref has reached + * 0). Holding a topology reference implies that a malloc reference will be + * held to @port as long as the user holds the topology reference. + * + * Care should be taken to ensure that the user has at least one malloc + * reference to @port. If you already have a topology reference to @port, you + * should use drm_dp_mst_topology_get_port() instead. + * + * See also: + * drm_dp_mst_topology_get_port() + * drm_dp_mst_topology_put_port() + * + * Returns: + * * 1: A topology reference was grabbed successfully + * * 0: @port is no longer in the topology, no reference was grabbed + */ +static int __must_check +drm_dp_mst_topology_try_get_port(struct drm_dp_mst_port *port) +{ + int ret = kref_get_unless_zero(&port->topology_kref); + + if (ret) + DRM_DEBUG("port %p (%d)\n", port, + kref_read(&port->topology_kref)); + + return ret; } +/** + * drm_dp_mst_topology_get_port() - Increment the topology refcount of a port + * @port: The &struct drm_dp_mst_port to increment the topology refcount of + * + * Increments &drm_dp_mst_port.topology_refcount without checking whether or + * not it's already reached 0. This is only valid to use in scenarios where + * you are already guaranteed to have at least one active topology reference + * to @port. Otherwise, drm_dp_mst_topology_try_get_port() must be used. + * + * See also: + * drm_dp_mst_topology_try_get_port() + * drm_dp_mst_topology_put_port() + */ +static void drm_dp_mst_topology_get_port(struct drm_dp_mst_port *port) +{ + WARN_ON(kref_read(&port->topology_kref) == 0); + kref_get(&port->topology_kref); + DRM_DEBUG("port %p (%d)\n", port, kref_read(&port->topology_kref)); +} + +/** + * drm_dp_mst_topology_put_port() - release a topology reference to a port + * @port: The &struct drm_dp_mst_port to release the topology reference from + * + * Releases a topology reference from @port by decrementing + * &drm_dp_mst_port.topology_kref. + * + * See also: + * drm_dp_mst_topology_try_get_port() + * drm_dp_mst_topology_get_port() + */ static void drm_dp_mst_topology_put_port(struct drm_dp_mst_port *port) { - kref_put(&port->kref, drm_dp_destroy_port); + DRM_DEBUG("port %p (%d)\n", + port, kref_read(&port->topology_kref) - 1); + kref_put(&port->topology_kref, drm_dp_destroy_port); } static struct drm_dp_mst_branch * @@ -981,10 +1283,10 @@ drm_dp_mst_topology_get_mstb_validated_locked(struct drm_dp_mst_branch *mstb, { struct drm_dp_mst_port *port; struct drm_dp_mst_branch *rmstb; - if (to_find == mstb) { - kref_get(&mstb->kref); + + if (to_find == mstb) return mstb; - } + list_for_each_entry(port, &mstb->ports, next) { if (port->mstb) { rmstb = drm_dp_mst_topology_get_mstb_validated_locked( @@ -1001,25 +1303,32 @@ drm_dp_mst_topology_get_mstb_validated(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_branch *mstb) { struct drm_dp_mst_branch *rmstb = NULL; + mutex_lock(&mgr->lock); - if (mgr->mst_primary) + if (mgr->mst_primary) { rmstb = drm_dp_mst_topology_get_mstb_validated_locked( mgr->mst_primary, mstb); + + if (rmstb && !drm_dp_mst_topology_try_get_mstb(rmstb)) + rmstb = NULL; + } mutex_unlock(&mgr->lock); return rmstb; } -static struct drm_dp_mst_port *drm_dp_mst_get_port_ref_locked(struct drm_dp_mst_branch *mstb, struct drm_dp_mst_port *to_find) +static struct drm_dp_mst_port * +drm_dp_mst_topology_get_port_validated_locked(struct drm_dp_mst_branch *mstb, + struct drm_dp_mst_port *to_find) { struct drm_dp_mst_port *port, *mport; list_for_each_entry(port, &mstb->ports, next) { - if (port == to_find) { - kref_get(&port->kref); + if (port == to_find) return port; - } + if (port->mstb) { - mport = drm_dp_mst_get_port_ref_locked(port->mstb, to_find); + mport = drm_dp_mst_topology_get_port_validated_locked( + port->mstb, to_find); if (mport) return mport; } @@ -1032,9 +1341,15 @@ drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port) { struct drm_dp_mst_port *rport = NULL; + mutex_lock(&mgr->lock); - if (mgr->mst_primary) - rport = drm_dp_mst_get_port_ref_locked(mgr->mst_primary, port); + if (mgr->mst_primary) { + rport = drm_dp_mst_topology_get_port_validated_locked( + mgr->mst_primary, port); + + if (rport && !drm_dp_mst_topology_try_get_port(rport)) + rport = NULL; + } mutex_unlock(&mgr->lock); return rport; } @@ -1042,11 +1357,12 @@ drm_dp_mst_topology_get_port_validated(struct drm_dp_mst_topology_mgr *mgr, static struct drm_dp_mst_port *drm_dp_get_port(struct drm_dp_mst_branch *mstb, u8 port_num) { struct drm_dp_mst_port *port; + int ret; list_for_each_entry(port, &mstb->ports, next) { if (port->port_num == port_num) { - kref_get(&port->kref); - return port; + ret = drm_dp_mst_topology_try_get_port(port); + return ret ? port : NULL; } } @@ -1095,6 +1411,11 @@ static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port) if (port->mstb) { port->mstb->mgr = port->mgr; port->mstb->port_parent = port; + /* + * Make sure this port's memory allocation stays + * around until it's child MSTB releases it + */ + drm_dp_mst_get_port_malloc(port); send_link = true; } @@ -1155,17 +1476,26 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, bool created = false; int old_pdt = 0; int old_ddps = 0; + port = drm_dp_get_port(mstb, port_msg->port_number); if (!port) { port = kzalloc(sizeof(*port), GFP_KERNEL); if (!port) return; - kref_init(&port->kref); + kref_init(&port->topology_kref); + kref_init(&port->malloc_kref); port->parent = mstb; port->port_num = port_msg->port_number; port->mgr = mstb->mgr; port->aux.name = "DPMST"; port->aux.dev = dev->dev; + + /* + * Make sure the memory allocation for our parent branch stays + * around until our own memory allocation is released + */ + drm_dp_mst_get_mstb_malloc(mstb); + created = true; } else { old_pdt = port->pdt; @@ -1185,7 +1515,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch *mstb, for this list */ if (created) { mutex_lock(&mstb->mgr->lock); - kref_get(&port->kref); + drm_dp_mst_topology_get_port(port); list_add(&port->next, &mstb->ports); mutex_unlock(&mstb->mgr->lock); } @@ -1284,7 +1614,7 @@ static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct drm_dp_mst_ { struct drm_dp_mst_branch *mstb; struct drm_dp_mst_port *port; - int i; + int i, ret; /* find the port by iterating down */ mutex_lock(&mgr->lock); @@ -1309,7 +1639,9 @@ static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct drm_dp_mst_ } } } - kref_get(&mstb->kref); + ret = drm_dp_mst_topology_try_get_mstb(mstb); + if (!ret) + mstb = NULL; out: mutex_unlock(&mgr->lock); return mstb; @@ -1344,14 +1676,17 @@ drm_dp_get_mst_branch_device_by_guid(struct drm_dp_mst_topology_mgr *mgr, uint8_t *guid) { struct drm_dp_mst_branch *mstb; + int ret; /* find the port by iterating down */ mutex_lock(&mgr->lock); mstb = get_mst_branch_device_by_guid_helper(mgr->mst_primary, guid); - - if (mstb) - kref_get(&mstb->kref); + if (mstb) { + ret = drm_dp_mst_topology_try_get_mstb(mstb); + if (!ret) + mstb = NULL; + } mutex_unlock(&mgr->lock); return mstb; @@ -1390,11 +1725,14 @@ static void drm_dp_mst_link_probe_work(struct work_struct *work) { struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, work); struct drm_dp_mst_branch *mstb; + int ret; mutex_lock(&mgr->lock); mstb = mgr->mst_primary; if (mstb) { - kref_get(&mstb->kref); + ret = drm_dp_mst_topology_try_get_mstb(mstb); + if (!ret) + mstb = NULL; } mutex_unlock(&mgr->lock); if (mstb) { @@ -1722,8 +2060,10 @@ static struct drm_dp_mst_branch *drm_dp_get_last_connected_port_and_mstb(struct if (found_port) { rmstb = found_port->parent; - kref_get(&rmstb->kref); - *port_num = found_port->port_num; + if (drm_dp_mst_topology_try_get_mstb(rmstb)) + *port_num = found_port->port_num; + else + rmstb = NULL; } } mutex_unlock(&mgr->lock); @@ -2176,7 +2516,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms /* give this the main reference */ mgr->mst_primary = mstb; - kref_get(&mgr->mst_primary->kref); + drm_dp_mst_topology_get_mstb(mgr->mst_primary); ret = drm_dp_dpcd_writeb(mgr->aux, DP_MSTM_CTRL, DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC); @@ -3096,13 +3436,6 @@ static void drm_dp_tx_work(struct work_struct *work) mutex_unlock(&mgr->qlock); } -static void drm_dp_free_mst_port(struct kref *kref) -{ - struct drm_dp_mst_port *port = container_of(kref, struct drm_dp_mst_port, kref); - kref_put(&port->parent->kref, drm_dp_free_mst_branch_device); - kfree(port); -} - static void drm_dp_destroy_connector_work(struct work_struct *work) { struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct drm_dp_mst_topology_mgr, destroy_connector_work); @@ -3123,7 +3456,6 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) list_del(&port->next); mutex_unlock(&mgr->destroy_connector_lock); - kref_init(&port->kref); INIT_LIST_HEAD(&port->next); mgr->cbs->destroy_connector(mgr, port->connector); @@ -3137,7 +3469,7 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi); } - kref_put(&port->kref, drm_dp_free_mst_port); + drm_dp_mst_put_port_malloc(port); send_hotplug = true; } if (send_hotplug) diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 371cc2816477..8eca5f29242c 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -44,7 +44,6 @@ struct drm_dp_vcpi { /** * struct drm_dp_mst_port - MST port - * @kref: reference count for this port. * @port_num: port number * @input: if this port is an input port. * @mcs: message capability status - DP 1.2 spec. @@ -67,7 +66,18 @@ struct drm_dp_vcpi { * in the MST topology. */ struct drm_dp_mst_port { - struct kref kref; + /** + * @topology_kref: refcount for this port's lifetime in the topology, + * only the DP MST helpers should need to touch this + */ + struct kref topology_kref; + + /** + * @malloc_kref: refcount for the memory allocation containing this + * structure. See drm_dp_mst_get_port_malloc() and + * drm_dp_mst_put_port_malloc(). + */ + struct kref malloc_kref; u8 port_num; bool input; @@ -102,7 +112,6 @@ struct drm_dp_mst_port { /** * struct drm_dp_mst_branch - MST branch device. - * @kref: reference count for this port. * @rad: Relative Address to talk to this branch device. * @lct: Link count total to talk to this branch device. * @num_ports: number of ports on the branch. @@ -121,7 +130,19 @@ struct drm_dp_mst_port { * to downstream port of parent branches. */ struct drm_dp_mst_branch { - struct kref kref; + /** + * @topology_kref: refcount for this branch device's lifetime in the + * topology, only the DP MST helpers should need to touch this + */ + struct kref topology_kref; + + /** + * @malloc_kref: refcount for the memory allocation containing this + * structure. See drm_dp_mst_get_mstb_malloc() and + * drm_dp_mst_put_mstb_malloc(). + */ + struct kref malloc_kref; + u8 rad[8]; u8 lct; int num_ports; @@ -626,4 +647,7 @@ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, bool power_up); +void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port); +void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port); + #endif -- cgit v1.2.3 From bea5c38f1eb66982d9506ece9d7a3941f339ac10 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Thu, 10 Jan 2019 19:53:40 -0500 Subject: drm/dp_mst: Add some atomic state iterator macros Changes since v6: - Move EXPORT_SYMBOL() for drm_dp_mst_topology_state_funcs to this commit - Document __drm_dp_mst_state_iter_get() and note that it shouldn't be called directly Signed-off-by: Lyude Paul Reviewed-by: Daniel Vetter Cc: David Airlie Cc: Jerry Zuo Cc: Harry Wentland Cc: Juston Li Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-18-lyude@redhat.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 5 +- include/drm/drm_dp_mst_helper.h | 96 +++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index b5976f8c318c..e3497bc49494 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3521,10 +3521,11 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, kfree(mst_state); } -static const struct drm_private_state_funcs mst_state_funcs = { +const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = { .atomic_duplicate_state = drm_dp_mst_duplicate_state, .atomic_destroy_state = drm_dp_mst_destroy_state, }; +EXPORT_SYMBOL(drm_dp_mst_topology_state_funcs); /** * drm_atomic_get_mst_topology_state: get MST topology state @@ -3608,7 +3609,7 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, drm_atomic_private_obj_init(dev, &mgr->base, &mst_state->base, - &mst_state_funcs); + &drm_dp_mst_topology_state_funcs); return 0; } diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 8eca5f29242c..581163c8d7d7 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -650,4 +650,100 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr, void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port); void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port); +extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs; + +/** + * __drm_dp_mst_state_iter_get - private atomic state iterator function for + * macro-internal use + * @state: &struct drm_atomic_state pointer + * @mgr: pointer to the &struct drm_dp_mst_topology_mgr iteration cursor + * @old_state: optional pointer to the old &struct drm_dp_mst_topology_state + * iteration cursor + * @new_state: optional pointer to the new &struct drm_dp_mst_topology_state + * iteration cursor + * @i: int iteration cursor, for macro-internal use + * + * Used by for_each_oldnew_mst_mgr_in_state(), + * for_each_old_mst_mgr_in_state(), and for_each_new_mst_mgr_in_state(). Don't + * call this directly. + * + * Returns: + * True if the current &struct drm_private_obj is a &struct + * drm_dp_mst_topology_mgr, false otherwise. + */ +static inline bool +__drm_dp_mst_state_iter_get(struct drm_atomic_state *state, + struct drm_dp_mst_topology_mgr **mgr, + struct drm_dp_mst_topology_state **old_state, + struct drm_dp_mst_topology_state **new_state, + int i) +{ + struct __drm_private_objs_state *objs_state = &state->private_objs[i]; + + if (objs_state->ptr->funcs != &drm_dp_mst_topology_state_funcs) + return false; + + *mgr = to_dp_mst_topology_mgr(objs_state->ptr); + if (old_state) + *old_state = to_dp_mst_topology_state(objs_state->old_state); + if (new_state) + *new_state = to_dp_mst_topology_state(objs_state->new_state); + + return true; +} + +/** + * for_each_oldnew_mst_mgr_in_state - iterate over all DP MST topology + * managers in an atomic update + * @__state: &struct drm_atomic_state pointer + * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor + * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old + * state + * @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new + * state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all DRM DP MST topology managers in an atomic update, + * tracking both old and new state. This is useful in places where the state + * delta needs to be considered, for example in atomic check functions. + */ +#define for_each_oldnew_mst_mgr_in_state(__state, mgr, old_state, new_state, __i) \ + for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \ + for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), &(old_state), &(new_state), (__i))) + +/** + * for_each_old_mst_mgr_in_state - iterate over all DP MST topology managers + * in an atomic update + * @__state: &struct drm_atomic_state pointer + * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor + * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old + * state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all DRM DP MST topology managers in an atomic update, + * tracking only the old state. This is useful in disable functions, where we + * need the old state the hardware is still in. + */ +#define for_each_old_mst_mgr_in_state(__state, mgr, old_state, __i) \ + for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \ + for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), &(old_state), NULL, (__i))) + +/** + * for_each_new_mst_mgr_in_state - iterate over all DP MST topology managers + * in an atomic update + * @__state: &struct drm_atomic_state pointer + * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor + * @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new + * state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all DRM DP MST topology managers in an atomic update, + * tracking only the new state. This is useful in enable functions, where we + * need the new state the hardware should be in when the atomic commit + * operation has completed. + */ +#define for_each_new_mst_mgr_in_state(__state, mgr, new_state, __i) \ + for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \ + for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr), NULL, &(new_state), (__i))) + #endif -- cgit v1.2.3 From eceae147246749c6dbaeefda802b30f804a3c54c Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Thu, 10 Jan 2019 19:53:41 -0500 Subject: drm/dp_mst: Start tracking per-port VCPI allocations There has been a TODO waiting for quite a long time in drm_dp_mst_topology.c: /* We cannot rely on port->vcpi.num_slots to update * topology_state->avail_slots as the port may not exist if the parent * branch device was unplugged. This should be fixed by tracking * per-port slot allocation in drm_dp_mst_topology_state instead of * depending on the caller to tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learned trying to implement fallback retraining in MST. So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for each port. This allows us to ensure idempotency without having to rely on the driver as much. Additionally: the driver doesn't need to do any kind of VCPI slot tracking anymore if it doesn't need it for it's own internal state. Additionally; this adds a new drm_dp_mst_atomic_check() helper which must be used by atomic drivers to perform validity checks for the new VCPI allocations incurred by a state. Also: update the documentation and make it more obvious that these /must/ be called by /all/ atomic drivers supporting MST. Changes since v9: * Add some missing changes that were requested by danvet that I forgot about after I redid all of the kref stuff: * Remove unnecessary state changes in intel_dp_mst_atomic_check * Cleanup atomic check logic for VCPI allocations - all we need to check in compute_config is whether or not this state disables a CRTC, then free VCPI based off that Changes since v8: * Fix compile errors, whoops! Changes since v7: - Don't check for mixed stale/valid VCPI allocations, just rely on connector registration to stop such erroneous modesets Changes since v6: - Keep a kref to all of the ports we have allocations on. This required a good bit of changing to when we call drm_dp_find_vcpi_slots(), mainly that we need to ensure that we only redo VCPI allocations on actual mode or CRTC changes, not crtc_state->active changes. Additionally, we no longer take the registration of the DRM connector for each port into account because so long as we have a kref to the port in the new or previous atomic state, the connector will stay registered. - Use the small changes to drm_dp_put_port() to add even more error checking to make misusage of the helpers more obvious. I added this after having to chase down various use-after-free conditions that started popping up from the new helpers so no one else has to troubleshoot that. - Move some accidental DRM_DEBUG_KMS() calls to DRM_DEBUG_ATOMIC() - Update documentation again, note that find/release() should both not be called on the same port in a single atomic check phase (but multiple calls to one or the other is OK) Changes since v4: - Don't skip the atomic checks for VCPI allocations if no new VCPI allocations happen in a state. This makes the next change I'm about to list here a lot easier to implement. - Don't ignore VCPI allocations on destroyed ports, instead ensure that when ports are destroyed and still have VCPI allocations in the topology state, the only state changes allowed are releasing said ports' VCPI. This prevents a state with a mix of VCPI allocations from destroyed ports, and allocations from valid ports. Changes since v3: - Don't release VCPI allocations in the topology state immediately in drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip over them in drm_dp_mst_duplicate_state(). This makes it so drm_dp_atomic_release_vcpi_slots() is still idempotent while also throwing warnings if the driver messes up it's book keeping and tries to release VCPI slots on a port that doesn't have any pre-existing VCPI allocation - danvet - Change mst_state/state in some debugging messages to "mst state" Changes since v2: - Use kmemdup() for duplicating MST state - danvet - Move port validation out of duplicate state callback - danvet - Handle looping through MST topology states in drm_dp_mst_atomic_check() so the driver doesn't have to do it - Fix documentation in drm_dp_atomic_find_vcpi_slots() - Move the atomic check for each individual topology state into it's own function, reduces indenting - Don't consider "stale" MST ports when calculating the bandwidth requirements. This is needed because originally we relied on the state duplication functions to prune any stale ports from the new state, which would prevent us from incorrectly considering their bandwidth requirements alongside legitimate new payloads. - Add function references in drm_dp_atomic_release_vcpi_slots() - danvet - Annotate atomic VCPI and atomic check functions with __must_check - danvet Changes since v1: - Don't use the now-removed ->atomic_check() for private objects hook, just give drivers a function to call themselves Signed-off-by: Lyude Paul Reviewed-by: Daniel Vetter Cc: David Airlie Cc: Jerry Zuo Cc: Harry Wentland Cc: Juston Li Link: https://patchwork.freedesktop.org/patch/msgid/20190111005343.17443-19-lyude@redhat.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 247 +++++++++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_dp_mst.c | 54 ++++---- include/drm/drm_dp_mst_helper.h | 23 +++- 4 files changed, 266 insertions(+), 62 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e3497bc49494..88db6d7e1a36 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3011,21 +3011,42 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, } /** - * drm_dp_atomic_find_vcpi_slots() - Find and add vcpi slots to the state + * drm_dp_atomic_find_vcpi_slots() - Find and add VCPI slots to the state * @state: global atomic state * @mgr: MST topology manager for the port * @port: port to find vcpi slots for * @pbn: bandwidth required for the mode in PBN * - * RETURNS: - * Total slots in the atomic state assigned for this port or error + * Allocates VCPI slots to @port, replacing any previous VCPI allocations it + * may have had. Any atomic drivers which support MST must call this function + * in their &drm_encoder_helper_funcs.atomic_check() callback to change the + * current VCPI allocation for the new state, but only when + * &drm_crtc_state.mode_changed or &drm_crtc_state.connectors_changed is set + * to ensure compatibility with userspace applications that still use the + * legacy modesetting UAPI. + * + * Allocations set by this function are not checked against the bandwidth + * restraints of @mgr until the driver calls drm_dp_mst_atomic_check(). + * + * Additionally, it is OK to call this function multiple times on the same + * @port as needed. It is not OK however, to call this function and + * drm_dp_atomic_release_vcpi_slots() in the same atomic check phase. + * + * See also: + * drm_dp_atomic_release_vcpi_slots() + * drm_dp_mst_atomic_check() + * + * Returns: + * Total slots in the atomic state assigned for this port, or a negative error + * code if the port no longer exists */ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn) { struct drm_dp_mst_topology_state *topology_state; - int req_slots; + struct drm_dp_vcpi_allocation *pos, *vcpi = NULL; + int prev_slots, req_slots, ret; topology_state = drm_atomic_get_mst_topology_state(state, mgr); if (IS_ERR(topology_state)) @@ -3034,20 +3055,54 @@ int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, port = drm_dp_mst_topology_get_port_validated(mgr, port); if (port == NULL) return -EINVAL; - req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); - DRM_DEBUG_KMS("vcpi slots req=%d, avail=%d\n", - req_slots, topology_state->avail_slots); - if (req_slots > topology_state->avail_slots) { - drm_dp_mst_topology_put_port(port); - return -ENOSPC; + /* Find the current allocation for this port, if any */ + list_for_each_entry(pos, &topology_state->vcpis, next) { + if (pos->port == port) { + vcpi = pos; + prev_slots = vcpi->vcpi; + + /* + * This should never happen, unless the driver tries + * releasing and allocating the same VCPI allocation, + * which is an error + */ + if (WARN_ON(!prev_slots)) { + DRM_ERROR("cannot allocate and release VCPI on [MST PORT:%p] in the same state\n", + port); + return -EINVAL; + } + + break; + } } + if (!vcpi) + prev_slots = 0; + + req_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); + + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] [MST PORT:%p] VCPI %d -> %d\n", + port->connector->base.id, port->connector->name, + port, prev_slots, req_slots); + + /* Add the new allocation to the state */ + if (!vcpi) { + vcpi = kzalloc(sizeof(*vcpi), GFP_KERNEL); + if (!vcpi) { + ret = -ENOMEM; + goto out; + } - topology_state->avail_slots -= req_slots; - DRM_DEBUG_KMS("vcpi slots avail=%d", topology_state->avail_slots); + drm_dp_mst_get_port_malloc(port); + vcpi->port = port; + list_add(&vcpi->next, &topology_state->vcpis); + } + vcpi->vcpi = req_slots; + ret = req_slots; +out: drm_dp_mst_topology_put_port(port); - return req_slots; + return ret; } EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots); @@ -3055,31 +3110,57 @@ EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots); * drm_dp_atomic_release_vcpi_slots() - Release allocated vcpi slots * @state: global atomic state * @mgr: MST topology manager for the port - * @slots: number of vcpi slots to release + * @port: The port to release the VCPI slots from * - * RETURNS: - * 0 if @slots were added back to &drm_dp_mst_topology_state->avail_slots or - * negative error code + * Releases any VCPI slots that have been allocated to a port in the atomic + * state. Any atomic drivers which support MST must call this function in + * their &drm_connector_helper_funcs.atomic_check() callback when the + * connector will no longer have VCPI allocated (e.g. because it's CRTC was + * removed) when it had VCPI allocated in the previous atomic state. + * + * It is OK to call this even if @port has been removed from the system. + * Additionally, it is OK to call this function multiple times on the same + * @port as needed. It is not OK however, to call this function and + * drm_dp_atomic_find_vcpi_slots() on the same @port in a single atomic check + * phase. + * + * See also: + * drm_dp_atomic_find_vcpi_slots() + * drm_dp_mst_atomic_check() + * + * Returns: + * 0 if all slots for this port were added back to + * &drm_dp_mst_topology_state.avail_slots or negative error code */ int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, - int slots) + struct drm_dp_mst_port *port) { struct drm_dp_mst_topology_state *topology_state; + struct drm_dp_vcpi_allocation *pos; + bool found = false; topology_state = drm_atomic_get_mst_topology_state(state, mgr); if (IS_ERR(topology_state)) return PTR_ERR(topology_state); - /* We cannot rely on port->vcpi.num_slots to update - * topology_state->avail_slots as the port may not exist if the parent - * branch device was unplugged. This should be fixed by tracking - * per-port slot allocation in drm_dp_mst_topology_state instead of - * depending on the caller to tell us how many slots to release. - */ - topology_state->avail_slots += slots; - DRM_DEBUG_KMS("vcpi slots released=%d, avail=%d\n", - slots, topology_state->avail_slots); + list_for_each_entry(pos, &topology_state->vcpis, next) { + if (pos->port == port) { + found = true; + break; + } + } + if (WARN_ON(!found)) { + DRM_ERROR("no VCPI for [MST PORT:%p] found in mst state %p\n", + port, &topology_state->base); + return -EINVAL; + } + + DRM_DEBUG_ATOMIC("[MST PORT:%p] VCPI %d -> 0\n", port, pos->vcpi); + if (pos->vcpi) { + drm_dp_mst_put_port_malloc(port); + pos->vcpi = 0; + } return 0; } @@ -3501,15 +3582,41 @@ static void drm_dp_destroy_connector_work(struct work_struct *work) static struct drm_private_state * drm_dp_mst_duplicate_state(struct drm_private_obj *obj) { - struct drm_dp_mst_topology_state *state; + struct drm_dp_mst_topology_state *state, *old_state = + to_dp_mst_topology_state(obj->state); + struct drm_dp_vcpi_allocation *pos, *vcpi; - state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL); + state = kmemdup(old_state, sizeof(*state), GFP_KERNEL); if (!state) return NULL; __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base); + INIT_LIST_HEAD(&state->vcpis); + + list_for_each_entry(pos, &old_state->vcpis, next) { + /* Prune leftover freed VCPI allocations */ + if (!pos->vcpi) + continue; + + vcpi = kmemdup(pos, sizeof(*vcpi), GFP_KERNEL); + if (!vcpi) + goto fail; + + drm_dp_mst_get_port_malloc(vcpi->port); + list_add(&vcpi->next, &state->vcpis); + } + return &state->base; + +fail: + list_for_each_entry_safe(pos, vcpi, &state->vcpis, next) { + drm_dp_mst_put_port_malloc(pos->port); + kfree(pos); + } + kfree(state); + + return NULL; } static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, @@ -3517,10 +3624,88 @@ static void drm_dp_mst_destroy_state(struct drm_private_obj *obj, { struct drm_dp_mst_topology_state *mst_state = to_dp_mst_topology_state(state); + struct drm_dp_vcpi_allocation *pos, *tmp; + + list_for_each_entry_safe(pos, tmp, &mst_state->vcpis, next) { + /* We only keep references to ports with non-zero VCPIs */ + if (pos->vcpi) + drm_dp_mst_put_port_malloc(pos->port); + kfree(pos); + } kfree(mst_state); } +static inline int +drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_topology_state *mst_state) +{ + struct drm_dp_vcpi_allocation *vcpi; + int avail_slots = 63; + + list_for_each_entry(vcpi, &mst_state->vcpis, next) { + /* Releasing VCPI is always OK-even if the port is gone */ + if (!vcpi->vcpi) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] releases all VCPI slots\n", + vcpi->port); + continue; + } + + DRM_DEBUG_ATOMIC("[MST PORT:%p] requires %d vcpi slots\n", + vcpi->port, vcpi->vcpi); + + avail_slots -= vcpi->vcpi; + if (avail_slots < 0) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] not enough VCPI slots in mst state %p (avail=%d)\n", + vcpi->port, mst_state, + avail_slots + vcpi->vcpi); + return -ENOSPC; + } + } + DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n", + mgr, mst_state, avail_slots, + 63 - avail_slots); + + return 0; +} + +/** + * drm_dp_mst_atomic_check - Check that the new state of an MST topology in an + * atomic update is valid + * @state: Pointer to the new &struct drm_dp_mst_topology_state + * + * Checks the given topology state for an atomic update to ensure that it's + * valid. This includes checking whether there's enough bandwidth to support + * the new VCPI allocations in the atomic update. + * + * Any atomic drivers supporting DP MST must make sure to call this after + * checking the rest of their state in their + * &drm_mode_config_funcs.atomic_check() callback. + * + * See also: + * drm_dp_atomic_find_vcpi_slots() + * drm_dp_atomic_release_vcpi_slots() + * + * Returns: + * + * 0 if the new state is valid, negative error code otherwise. + */ +int drm_dp_mst_atomic_check(struct drm_atomic_state *state) +{ + struct drm_dp_mst_topology_mgr *mgr; + struct drm_dp_mst_topology_state *mst_state; + int i, ret = 0; + + for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { + ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); + if (ret) + break; + } + + return ret; +} +EXPORT_SYMBOL(drm_dp_mst_atomic_check); + const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs = { .atomic_duplicate_state = drm_dp_mst_duplicate_state, .atomic_destroy_state = drm_dp_mst_destroy_state, @@ -3603,9 +3788,7 @@ int drm_dp_mst_topology_mgr_init(struct drm_dp_mst_topology_mgr *mgr, return -ENOMEM; mst_state->mgr = mgr; - - /* max. time slots - one slot for MTP header */ - mst_state->avail_slots = 63; + INIT_LIST_HEAD(&mst_state->vcpis); drm_atomic_private_obj_init(dev, &mgr->base, &mst_state->base, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 812ec5ae5c7b..5a679af03a04 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12623,6 +12623,10 @@ static int intel_atomic_check(struct drm_device *dev, "[modeset]" : "[fastset]"); } + ret = drm_dp_mst_atomic_check(state); + if (ret) + return ret; + if (any_ms) { ret = intel_modeset_checks(state); diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 631fd1537252..c8e2215628e6 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -41,8 +41,12 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, struct drm_connector *connector = conn_state->connector; void *port = to_intel_connector(connector)->port; struct drm_atomic_state *state = pipe_config->base.state; + struct drm_crtc *crtc = pipe_config->base.crtc; + struct drm_crtc_state *old_crtc_state = + drm_atomic_get_old_crtc_state(state, crtc); int bpp; - int lane_count, slots = 0; + int lane_count, slots = + to_intel_crtc_state(old_crtc_state)->dp_m_n.tu; const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; int mst_pbn; bool constant_n = drm_dp_has_quirk(&intel_dp->desc, @@ -107,35 +111,39 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder, return true; } -static int intel_dp_mst_atomic_check(struct drm_connector *connector, - struct drm_connector_state *new_conn_state) +static int +intel_dp_mst_atomic_check(struct drm_connector *connector, + struct drm_connector_state *new_conn_state) { struct drm_atomic_state *state = new_conn_state->state; - struct drm_connector_state *old_conn_state; - struct drm_crtc *old_crtc; + struct drm_connector_state *old_conn_state = + drm_atomic_get_old_connector_state(state, connector); + struct intel_connector *intel_connector = + to_intel_connector(connector); + struct drm_crtc *new_crtc = new_conn_state->crtc; struct drm_crtc_state *crtc_state; - int slots, ret = 0; + struct drm_dp_mst_topology_mgr *mgr; + int ret = 0; - old_conn_state = drm_atomic_get_old_connector_state(state, connector); - old_crtc = old_conn_state->crtc; - if (!old_crtc) - return ret; - - crtc_state = drm_atomic_get_new_crtc_state(state, old_crtc); - slots = to_intel_crtc_state(crtc_state)->dp_m_n.tu; - if (drm_atomic_crtc_needs_modeset(crtc_state) && slots > 0) { - struct drm_dp_mst_topology_mgr *mgr; - struct drm_encoder *old_encoder; + if (!old_conn_state->crtc) + return 0; - old_encoder = old_conn_state->best_encoder; - mgr = &enc_to_mst(old_encoder)->primary->dp.mst_mgr; + /* We only want to free VCPI if this state disables the CRTC on this + * connector + */ + if (new_crtc) { + crtc_state = drm_atomic_get_new_crtc_state(state, new_crtc); - ret = drm_dp_atomic_release_vcpi_slots(state, mgr, slots); - if (ret) - DRM_DEBUG_KMS("failed releasing %d vcpi slots:%d\n", slots, ret); - else - to_intel_crtc_state(crtc_state)->dp_m_n.tu = 0; + if (!crtc_state || + !drm_atomic_crtc_needs_modeset(crtc_state) || + crtc_state->enable) + return 0; } + + mgr = &enc_to_mst(old_conn_state->best_encoder)->primary->dp.mst_mgr; + ret = drm_dp_atomic_release_vcpi_slots(state, mgr, + intel_connector->port); + return ret; } diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 581163c8d7d7..451d020f0137 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -425,9 +425,15 @@ struct drm_dp_payload { #define to_dp_mst_topology_state(x) container_of(x, struct drm_dp_mst_topology_state, base) +struct drm_dp_vcpi_allocation { + struct drm_dp_mst_port *port; + int vcpi; + struct list_head next; +}; + struct drm_dp_mst_topology_state { struct drm_private_state base; - int avail_slots; + struct list_head vcpis; struct drm_dp_mst_topology_mgr *mgr; }; @@ -638,14 +644,17 @@ void drm_dp_mst_topology_mgr_suspend(struct drm_dp_mst_topology_mgr *mgr); int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr); struct drm_dp_mst_topology_state *drm_atomic_get_mst_topology_state(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr); -int drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr, - struct drm_dp_mst_port *port, int pbn); -int drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, - struct drm_dp_mst_topology_mgr *mgr, - int slots); +int __must_check +drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, + struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_port *port, int pbn); +int __must_check +drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, + struct drm_dp_mst_topology_mgr *mgr, + struct drm_dp_mst_port *port); int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, bool power_up); +int __must_check drm_dp_mst_atomic_check(struct drm_atomic_state *state); void drm_dp_mst_get_port_malloc(struct drm_dp_mst_port *port); void drm_dp_mst_put_port_malloc(struct drm_dp_mst_port *port); -- cgit v1.2.3 From 1e9080ac21d182913df404d634a7c847af5e35be Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 17 Dec 2018 20:42:59 +0100 Subject: drm: Unexport drm_crtc_force_disable It's a legacy kms only thing, good to hide it better now that all those old drivers use the legacy crtc helpers directly. Reviewed-by: Alex Deucher Signed-off-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Link: https://patchwork.freedesktop.org/patch/msgid/20181217194303.14397-3-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_crtc.c | 10 ---------- drivers/gpu/drm/drm_crtc_internal.h | 1 + include/drm/drm_crtc.h | 1 - 3 files changed, 1 insertion(+), 11 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 42cdb4181643..e224b9b7d17a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -93,15 +93,6 @@ struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx) } EXPORT_SYMBOL(drm_crtc_from_index); -/** - * drm_crtc_force_disable - Forcibly turn off a CRTC - * @crtc: CRTC to turn off - * - * Note: This should only be used by non-atomic legacy drivers. - * - * Returns: - * Zero on success, error code on failure. - */ int drm_crtc_force_disable(struct drm_crtc *crtc) { struct drm_mode_set set = { @@ -112,7 +103,6 @@ int drm_crtc_force_disable(struct drm_crtc *crtc) return drm_mode_set_config_internal(&set); } -EXPORT_SYMBOL(drm_crtc_force_disable); /** * drm_crtc_force_disable_all - Forcibly turn off all enabled CRTCs diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index 86893448f486..216f2a9ee3d4 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -50,6 +50,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc, const struct drm_framebuffer *fb); int drm_crtc_register_all(struct drm_device *dev); void drm_crtc_unregister_all(struct drm_device *dev); +int drm_crtc_force_disable(struct drm_crtc *crtc); struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b21437bc95bf..b955ef1f1e3e 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1140,7 +1140,6 @@ static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc) return 1 << drm_crtc_index(crtc); } -int drm_crtc_force_disable(struct drm_crtc *crtc); int drm_crtc_force_disable_all(struct drm_device *dev); int drm_mode_set_config_internal(struct drm_mode_set *set); -- cgit v1.2.3 From 7d0250ed8e69fb6a66caecf60b8753a21224cc1a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 12 Oct 2018 09:34:41 +0200 Subject: drm/atomic: Add missing () to function ref in kerneldoc Pure drive-by while reading code&docs. Signed-off-by: Daniel Vetter Reviewed-by: Lyude Paul Link: https://patchwork.freedesktop.org/patch/msgid/20181012073441.21774-1-daniel.vetter@ffwll.ch --- include/drm/drm_atomic.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index cac4a1b6b0e8..811b4a92568f 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -139,9 +139,9 @@ struct drm_crtc_commit { /** * @abort_completion: * - * A flag that's set after drm_atomic_helper_setup_commit takes a second - * reference for the completion of $drm_crtc_state.event. It's used by - * the free code to remove the second reference if commit fails. + * A flag that's set after drm_atomic_helper_setup_commit() takes a + * second reference for the completion of $drm_crtc_state.event. It's + * used by the free code to remove the second reference if commit fails. */ bool abort_completion; }; -- cgit v1.2.3 From c2d88e06bcb98540bb83fac874574eaa4f320363 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 17 Dec 2018 20:43:00 +0100 Subject: drm: Move the legacy kms disable_all helper to crtc helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's not a core function, and the matching atomic functions are also not in the core. Plus the suspend/resume helper is also already there. Needs a tiny bit of open-coding, but less midlayer beats that I think. v2: Rebase onto ast (which gained a new user). Cc: Sam Bobroff Reviewed-by: Alex Deucher Reviewed-by: Sean Paul Signed-off-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Ben Skeggs Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: Rex Zhu Cc: Andrey Grodzovsky Cc: Huang Rui Cc: Shaoyun Liu Cc: Monk Liu Cc: nouveau@lists.freedesktop.org Cc: amd-gfx@lists.freedesktop.org Link: https://patchwork.freedesktop.org/patch/msgid/20181217194303.14397-4-daniel.vetter@ffwll.ch --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- drivers/gpu/drm/ast/ast_fb.c | 2 +- drivers/gpu/drm/drm_crtc.c | 31 -------------------------- drivers/gpu/drm/drm_crtc_helper.c | 35 ++++++++++++++++++++++++++++++ drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- drivers/gpu/drm/radeon/radeon_display.c | 2 +- include/drm/drm_crtc.h | 2 -- include/drm/drm_crtc_helper.h | 1 + 8 files changed, 40 insertions(+), 37 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 8a078f4ae73d..28bccceaa363 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2708,7 +2708,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_irq_disable_all(adev); if (adev->mode_info.mode_config_initialized){ if (!amdgpu_device_has_dc_support(adev)) - drm_crtc_force_disable_all(adev->ddev); + drm_helper_force_disable_all(adev->ddev); else drm_atomic_helper_shutdown(adev->ddev); } diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c index c2e41369adcf..75f867d00031 100644 --- a/drivers/gpu/drm/ast/ast_fb.c +++ b/drivers/gpu/drm/ast/ast_fb.c @@ -261,7 +261,7 @@ static void ast_fbdev_destroy(struct drm_device *dev, { struct ast_framebuffer *afb = &afbdev->afb; - drm_crtc_force_disable_all(dev); + drm_helper_force_disable_all(dev); drm_fb_helper_unregister_fbi(&afbdev->helper); if (afb->obj) { diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f660819d406e..7dabbaf033a1 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -104,37 +104,6 @@ int drm_crtc_force_disable(struct drm_crtc *crtc) return drm_mode_set_config_internal(&set); } -/** - * drm_crtc_force_disable_all - Forcibly turn off all enabled CRTCs - * @dev: DRM device whose CRTCs to turn off - * - * Drivers may want to call this on unload to ensure that all displays are - * unlit and the GPU is in a consistent, low power state. Takes modeset locks. - * - * Note: This should only be used by non-atomic legacy drivers. For an atomic - * version look at drm_atomic_helper_shutdown(). - * - * Returns: - * Zero on success, error code on failure. - */ -int drm_crtc_force_disable_all(struct drm_device *dev) -{ - struct drm_crtc *crtc; - int ret = 0; - - drm_modeset_lock_all(dev); - drm_for_each_crtc(crtc, dev) - if (crtc->enabled) { - ret = drm_crtc_force_disable(crtc); - if (ret) - goto out; - } -out: - drm_modeset_unlock_all(dev); - return ret; -} -EXPORT_SYMBOL(drm_crtc_force_disable_all); - static unsigned int drm_num_crtcs(struct drm_device *dev) { unsigned int num = 0; diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 3b5cf93233c1..91dc9b6386e7 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -994,3 +994,38 @@ void drm_helper_resume_force_mode(struct drm_device *dev) drm_modeset_unlock_all(dev); } EXPORT_SYMBOL(drm_helper_resume_force_mode); + +/** + * drm_helper_force_disable_all - Forcibly turn off all enabled CRTCs + * @dev: DRM device whose CRTCs to turn off + * + * Drivers may want to call this on unload to ensure that all displays are + * unlit and the GPU is in a consistent, low power state. Takes modeset locks. + * + * Note: This should only be used by non-atomic legacy drivers. For an atomic + * version look at drm_atomic_helper_shutdown(). + * + * Returns: + * Zero on success, error code on failure. + */ +int drm_helper_force_disable_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + int ret = 0; + + drm_modeset_lock_all(dev); + drm_for_each_crtc(crtc, dev) + if (crtc->enabled) { + struct drm_mode_set set = { + .crtc = crtc, + }; + + ret = drm_mode_set_config_internal(&set); + if (ret) + goto out; + } +out: + drm_modeset_unlock_all(dev); + return ret; +} +EXPORT_SYMBOL(drm_helper_force_disable_all); diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index f326ffd86766..5d273a655479 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -453,7 +453,7 @@ nouveau_display_fini(struct drm_device *dev, bool suspend, bool runtime) if (drm_drv_uses_atomic_modeset(dev)) drm_atomic_helper_shutdown(dev); else - drm_crtc_force_disable_all(dev); + drm_helper_force_disable_all(dev); } /* disable flip completion events */ diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 9d3ac8b981da..b190478ad087 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1646,7 +1646,7 @@ void radeon_modeset_fini(struct radeon_device *rdev) if (rdev->mode_info.mode_config_initialized) { drm_kms_helper_poll_fini(rdev->ddev); radeon_hpd_fini(rdev); - drm_crtc_force_disable_all(rdev->ddev); + drm_helper_force_disable_all(rdev->ddev); radeon_fbdev_fini(rdev); radeon_afmt_fini(rdev); drm_mode_config_cleanup(rdev->ddev); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b45bec0b7a9c..85abd3fe9e83 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1149,8 +1149,6 @@ static inline uint32_t drm_crtc_mask(const struct drm_crtc *crtc) return 1 << drm_crtc_index(crtc); } -int drm_crtc_force_disable_all(struct drm_device *dev); - int drm_mode_set_config_internal(struct drm_mode_set *set); struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx); diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h index d65f034843ce..0ee9a96b70da 100644 --- a/include/drm/drm_crtc_helper.h +++ b/include/drm/drm_crtc_helper.h @@ -56,6 +56,7 @@ bool drm_helper_encoder_in_use(struct drm_encoder *encoder); int drm_helper_connector_dpms(struct drm_connector *connector, int mode); void drm_helper_resume_force_mode(struct drm_device *dev); +int drm_helper_force_disable_all(struct drm_device *dev); /* drm_probe_helper.c */ int drm_helper_probe_single_connector_modes(struct drm_connector -- cgit v1.2.3 From 3214a16684468f5429469c3b59dc25b312b9d6cc Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 11 Jan 2019 17:40:48 +0100 Subject: drm/doc: Polish kerneldoc for drm_device.h - Move all the legacy gunk at the bottom, and exclude it from kerneldoc. - Documentation for the remaining bits. v2: Fix typo (Sam). Cc: Sam Ravnborg Acked-by: Sam Ravnborg Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190111164048.29067-5-daniel.vetter@ffwll.ch --- Documentation/gpu/drm-internals.rst | 3 + include/drm/drm_device.h | 150 ++++++++++++++++++++---------------- 2 files changed, 88 insertions(+), 65 deletions(-) (limited to 'include') diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index 5ee9674fb9e9..9090fabf3f7a 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -143,6 +143,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..a6f8e5abb66f 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: Used 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 -- cgit v1.2.3 From e9eafcb589213395232084a2378e2e90f67feb29 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Sat, 12 Jan 2019 20:32:44 +0100 Subject: drm: move drm_can_sleep() to drm_util.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move drm_can_sleep() out of drmP.h to allow users to get rid of the drmP.h include. There was no header file that was a good match for this helper function. So add this to drm_util with the relevant includes. Add include of drm_util.h to all users. v2: - Update comments to use kernel-doc style (Daniel) - Add FIXME to drm_can_sleep and add note that this function should not be used in new code (Daniel) v3: - Fix kernel-doc syntax (Daniel) - Plug drm_util.h into drm-internels.rst (Daniel) Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Cc: Alex Deucher Cc: "Christian König" Cc: "David (ChunMing) Zhou" Cc: Gerd Hoffmann Cc: Rob Clark Cc: Tomi Valkeinen Cc: Eric Anholt Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190112193251.20450-2-sam@ravnborg.org --- Documentation/gpu/drm-internals.rst | 9 ++++++ drivers/gpu/drm/amd/amdgpu/atom.c | 2 ++ drivers/gpu/drm/ast/ast_fb.c | 2 ++ drivers/gpu/drm/cirrus/cirrus_fbdev.c | 1 + drivers/gpu/drm/drm_flip_work.c | 1 + drivers/gpu/drm/mgag200/mgag200_fb.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 1 + drivers/gpu/drm/omapdrm/omap_fbdev.c | 1 + drivers/gpu/drm/qxl/qxl_cmd.c | 2 ++ drivers/gpu/drm/radeon/atom.c | 2 ++ drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 1 + drivers/gpu/drm/vc4/vc4_drv.h | 1 + include/drm/drmP.h | 8 ----- include/drm/drm_util.h | 43 ++++++++++++++++++++++++- 14 files changed, 66 insertions(+), 9 deletions(-) (limited to 'include') diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index 9090fabf3f7a..2caf21effd28 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -233,6 +233,15 @@ Printer .. kernel-doc:: drivers/gpu/drm/drm_print.c :export: +Utilities +--------- + +.. kernel-doc:: include/drm/drm_util.h + :doc: drm utils + +.. kernel-doc:: include/drm/drm_util.h + :internal: + Legacy Support Code =================== diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c index e9934de1b9cf..dd30f4e61a8c 100644 --- a/drivers/gpu/drm/amd/amdgpu/atom.c +++ b/drivers/gpu/drm/amd/amdgpu/atom.c @@ -27,6 +27,8 @@ #include #include +#include + #define ATOM_DEBUG #include "atom.h" diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c index 75f867d00031..2c9f8dd9733a 100644 --- a/drivers/gpu/drm/ast/ast_fb.c +++ b/drivers/gpu/drm/ast/ast_fb.c @@ -39,7 +39,9 @@ #include #include #include +#include #include + #include "ast_drv.h" static void ast_dirty_update(struct ast_fbdev *afbdev, diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c index 3d0a3958d083..39df62acac69 100644 --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c @@ -10,6 +10,7 @@ */ #include #include +#include #include #include diff --git a/drivers/gpu/drm/drm_flip_work.c b/drivers/gpu/drm/drm_flip_work.c index 12dea16f22a8..3da3bf5af405 100644 --- a/drivers/gpu/drm/drm_flip_work.c +++ b/drivers/gpu/drm/drm_flip_work.c @@ -22,6 +22,7 @@ */ #include +#include #include /** diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c b/drivers/gpu/drm/mgag200/mgag200_fb.c index 30726c9fe28c..6893934b26c0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_fb.c +++ b/drivers/gpu/drm/mgag200/mgag200_fb.c @@ -12,6 +12,7 @@ */ #include #include +#include #include #include diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c index 7cebcb2b3a37..6153514db04c 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c @@ -16,6 +16,7 @@ * this program. If not, see . */ +#include #include "mdp5_kms.h" #include "mdp5_smp.h" diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c index aee99194499f..851c59f07eb1 100644 --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c @@ -16,6 +16,7 @@ */ #include +#include #include #include "omap_drv.h" diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index dffc5093ff16..2e100f644236 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -25,6 +25,8 @@ /* QXL cmd/ring handling */ +#include + #include "qxl_drv.h" #include "qxl_object.h" diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c index e55cbeee7a53..ac98ad561870 100644 --- a/drivers/gpu/drm/radeon/atom.c +++ b/drivers/gpu/drm/radeon/atom.c @@ -27,6 +27,8 @@ #include #include +#include + #define ATOM_DEBUG #include "atom.h" diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c index 222a1fa41d7c..7e3257e8fd56 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c @@ -24,6 +24,7 @@ * Alex Deucher */ #include +#include #include #include #include "radeon.h" diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index c24b078f0593..2c635f001c71 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 9e47c8dc6b87..bc4cb3732407 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -94,14 +94,6 @@ struct dma_buf_attachment; struct pci_dev; struct pci_controller; -/* returns true if currently okay to sleep */ -static inline bool drm_can_sleep(void) -{ - if (in_atomic() || in_dbg_master() || irqs_disabled()) - return false; - return true; -} - #if defined(CONFIG_DRM_DEBUG_SELFTEST_MODULE) #define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) EXPORT_SYMBOL(x) #else diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h index 88abdca89baa..f776a55e5508 100644 --- a/include/drm/drm_util.h +++ b/include/drm/drm_util.h @@ -26,7 +26,48 @@ #ifndef _DRM_UTIL_H_ #define _DRM_UTIL_H_ -/* helper for handling conditionals in various for_each macros */ +/** + * DOC: drm utils + * + * Macros and inline functions that does not naturally belong in other places + */ + +#include +#include +#include +#include + +/** + * for_each_if - helper for handling conditionals in various for_each macros + * @condition: The condition to check + * + * Typical use:: + * + * #define for_each_foo_bar(x, y) \' + * list_for_each_entry(x, y->list, head) \' + * for_each_if(x->something == SOMETHING) + * + * The for_each_if() macro makes the use of for_each_foo_bar() less error + * prone. + */ #define for_each_if(condition) if (!(condition)) {} else +/** + * drm_can_sleep - returns true if currently okay to sleep + * + * This function shall not be used in new code. + * The check for running in atomic context may not work - see linux/preempt.h. + * + * FIXME: All users of drm_can_sleep should be removed (see todo.rst) + * + * Returns: + * True if kgdb is active or we are in an atomic context or irqs are disabled + */ +static inline bool drm_can_sleep(void) +{ + if (in_atomic() || in_dbg_master() || irqs_disabled()) + return false; + return true; +} + #endif -- cgit v1.2.3 From 21376e2c3c5bad5e87ba700c055c8a8235c2bfd5 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Sat, 12 Jan 2019 20:32:45 +0100 Subject: drm: move EXPORT_SYMBOL_FOR_TESTS_ONLY to drm_util.h In the quest to get rid of drmP.h move the newly added EXPORT_SYMBOL_FOR_TESTS_ONLY to drm_util.h. Fix the single user. Add a note to drmP.h to avoid further use of it. Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190112193251.20450-3-sam@ravnborg.org --- drivers/gpu/drm/drm_framebuffer.c | 1 + include/drm/drmP.h | 11 ++++++----- include/drm/drm_util.h | 10 ++++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index fcaea8f50513..7abcb265a108 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "drm_internal.h" #include "drm_crtc_internal.h" diff --git a/include/drm/drmP.h b/include/drm/drmP.h index bc4cb3732407..3f5c577c9dbd 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -94,10 +94,11 @@ struct dma_buf_attachment; struct pci_dev; struct pci_controller; -#if defined(CONFIG_DRM_DEBUG_SELFTEST_MODULE) -#define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) EXPORT_SYMBOL(x) -#else -#define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) -#endif +/* + * NOTE: drmP.h is obsolete - do NOT add anything to this file + * + * Do not include drmP.h in new files. + * Work is ongoing to remove drmP.h includes from existing files + */ #endif diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h index f776a55e5508..0500da65b1d1 100644 --- a/include/drm/drm_util.h +++ b/include/drm/drm_util.h @@ -37,6 +37,16 @@ #include #include +/* + * Use EXPORT_SYMBOL_FOR_TESTS_ONLY() for functions that shall + * only be visible for drmselftests. + */ +#if defined(CONFIG_DRM_DEBUG_SELFTEST_MODULE) +#define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) EXPORT_SYMBOL(x) +#else +#define EXPORT_SYMBOL_FOR_TESTS_ONLY(x) +#endif + /** * for_each_if - helper for handling conditionals in various for_each macros * @condition: The condition to check -- cgit v1.2.3 From ed20151a7699bb2c77eba3610199789a126940c4 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Tue, 27 Nov 2018 20:20:04 +0200 Subject: drm/vblank: Allow dynamic per-crtc max_vblank_count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On i965gm we need to adjust max_vblank_count dynamically depending on whether the TV encoder is used or not. To that end add a per-crtc max_vblank_count that takes precedence over its device wide counterpart. The driver can now call drm_crtc_set_max_vblank_count() to configure the per-crtc value before calling drm_vblank_on(). Also looks like there was some discussion about exynos needing similar treatment. v2: Drop the extra max_vblank_count!=0 check for the WARN(last!=current), will take care of it in i915 code (Daniel) WARN_ON(!inmodeset) (Daniel) WARN_ON(dev->max_vblank_count) Pimp up the docs (Daniel) Cc: stable@vger.kernel.org Cc: Inki Dae Cc: Daniel Vetter Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181127182004.28885-1-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/drm_vblank.c | 45 +++++++++++++++++++++++++++++++++++++++++--- include/drm/drm_device.h | 6 ++++++ include/drm/drm_vblank.h | 22 ++++++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 98e091175921..cde71ee95a8f 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -105,13 +105,20 @@ static void store_vblank(struct drm_device *dev, unsigned int pipe, write_sequnlock(&vblank->seqlock); } +static u32 drm_max_vblank_count(struct drm_device *dev, unsigned int pipe) +{ + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; + + return vblank->max_vblank_count ?: dev->max_vblank_count; +} + /* * "No hw counter" fallback implementation of .get_vblank_counter() hook, * if there is no useable hardware frame counter available. */ static u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe) { - WARN_ON_ONCE(dev->max_vblank_count != 0); + WARN_ON_ONCE(drm_max_vblank_count(dev, pipe) != 0); return 0; } @@ -198,6 +205,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, ktime_t t_vblank; int count = DRM_TIMESTAMP_MAXRETRIES; int framedur_ns = vblank->framedur_ns; + u32 max_vblank_count = drm_max_vblank_count(dev, pipe); /* * Interrupts were disabled prior to this call, so deal with counter @@ -216,9 +224,9 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, in_vblank_irq); } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0); - if (dev->max_vblank_count != 0) { + if (max_vblank_count) { /* trust the hw counter when it's around */ - diff = (cur_vblank - vblank->last) & dev->max_vblank_count; + diff = (cur_vblank - vblank->last) & max_vblank_count; } else if (rc && framedur_ns) { u64 diff_ns = ktime_to_ns(ktime_sub(t_vblank, vblank->time)); @@ -1204,6 +1212,37 @@ void drm_crtc_vblank_reset(struct drm_crtc *crtc) } EXPORT_SYMBOL(drm_crtc_vblank_reset); +/** + * drm_crtc_set_max_vblank_count - configure the hw max vblank counter value + * @crtc: CRTC in question + * @max_vblank_count: max hardware vblank counter value + * + * Update the maximum hardware vblank counter value for @crtc + * at runtime. Useful for hardware where the operation of the + * hardware vblank counter depends on the currently active + * display configuration. + * + * For example, if the hardware vblank counter does not work + * when a specific connector is active the maximum can be set + * to zero. And when that specific connector isn't active the + * maximum can again be set to the appropriate non-zero value. + * + * If used, must be called before drm_vblank_on(). + */ +void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc, + u32 max_vblank_count) +{ + struct drm_device *dev = crtc->dev; + unsigned int pipe = drm_crtc_index(crtc); + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; + + WARN_ON(dev->max_vblank_count); + WARN_ON(!READ_ONCE(vblank->inmodeset)); + + vblank->max_vblank_count = max_vblank_count; +} +EXPORT_SYMBOL(drm_crtc_set_max_vblank_count); + /** * drm_crtc_vblank_on - enable vblank events on a CRTC * @crtc: CRTC in question diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index a6f8e5abb66f..d5e092dccf3e 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -240,6 +240,12 @@ struct drm_device { * races and imprecision over longer time periods, hence exposing a * hardware vblank counter is always recommended. * + * This is the statically configured device wide maximum. The driver + * can instead choose to use a runtime configurable per-crtc value + * &drm_vblank_crtc.max_vblank_count, in which case @max_vblank_count + * must be left at zero. See drm_crtc_set_max_vblank_count() on how + * to use the per-crtc value. + * * If non-zero, &drm_crtc_funcs.get_vblank_counter must be set. */ u32 max_vblank_count; diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h index 6ad9630d4f48..e528bb2f659d 100644 --- a/include/drm/drm_vblank.h +++ b/include/drm/drm_vblank.h @@ -128,6 +128,26 @@ struct drm_vblank_crtc { * @last: Protected by &drm_device.vbl_lock, used for wraparound handling. */ u32 last; + /** + * @max_vblank_count: + * + * Maximum value of the vblank registers for this crtc. This value +1 + * will result in a wrap-around of the vblank register. It is used + * by the vblank core to handle wrap-arounds. + * + * If set to zero the vblank core will try to guess the elapsed vblanks + * between times when the vblank interrupt is disabled through + * high-precision timestamps. That approach is suffering from small + * races and imprecision over longer time periods, hence exposing a + * hardware vblank counter is always recommended. + * + * This is the runtime configurable per-crtc maximum set through + * drm_crtc_set_max_vblank_count(). If this is used the driver + * must leave the device wide &drm_device.max_vblank_count at zero. + * + * If non-zero, &drm_crtc_funcs.get_vblank_counter must be set. + */ + u32 max_vblank_count; /** * @inmodeset: Tracks whether the vblank is disabled due to a modeset. * For legacy driver bit 2 additionally tracks whether an additional @@ -206,4 +226,6 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode); wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc); +void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc, + u32 max_vblank_count); #endif -- cgit v1.2.3 From 2db8ebca1f6c158d0d439ebec4d4b9924c4c3d3c Mon Sep 17 00:00:00 2001 From: Matteo Franchin Date: Wed, 9 Jan 2019 15:58:37 +0000 Subject: drm/fourcc: Add modifier defininitions for AFBC 1.3 This commit adds definitions of format modifiers for version 1.3 of the Arm Framebuffer Compression (AFBC). Signed-off-by: Matteo Franchin Signed-off-by: Ayan Kumar Halder Reviewed-by: Brian Starkey Link: https://patchwork.freedesktop.org/patch/277333/ --- include/uapi/drm/drm_fourcc.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'include') diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 0b44260a5ee9..41106c835747 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -581,10 +581,18 @@ extern "C" { * Indicates the superblock size(s) used for the AFBC buffer. The buffer * size (in pixels) must be aligned to a multiple of the superblock size. * Four lowest significant bits(LSBs) are reserved for block size. + * + * Where one superblock size is specified, it applies to all planes of the + * buffer (e.g. 16x16, 32x8). When multiple superblock sizes are specified, + * the first applies to the Luma plane and the second applies to the Chroma + * plane(s). e.g. (32x8_64x4 means 32x8 Luma, with 64x4 Chroma). + * Multiple superblock sizes are only valid for multi-plane YCbCr formats. */ #define AFBC_FORMAT_MOD_BLOCK_SIZE_MASK 0xf #define AFBC_FORMAT_MOD_BLOCK_SIZE_16x16 (1ULL) #define AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 (2ULL) +#define AFBC_FORMAT_MOD_BLOCK_SIZE_64x4 (3ULL) +#define AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4 (4ULL) /* * AFBC lossless colorspace transform @@ -644,6 +652,21 @@ extern "C" { */ #define AFBC_FORMAT_MOD_SC (1ULL << 9) +/* + * AFBC double-buffer + * + * Indicates that the buffer is allocated in a layout safe for front-buffer + * rendering. + */ +#define AFBC_FORMAT_MOD_DB (1ULL << 10) + +/* + * AFBC buffer content hints + * + * Indicates that the buffer includes per-superblock content hints. + */ +#define AFBC_FORMAT_MOD_BCH (1ULL << 11) + #if defined(__cplusplus) } #endif -- cgit v1.2.3 From a3004db01f85c3f6dc400c9dc663d91b9b3899b4 Mon Sep 17 00:00:00 2001 From: Shayenne Moura Date: Fri, 11 Jan 2019 12:47:29 -0200 Subject: drm: Complete remove drm_mode_object dependency This patch finalizes the KMS cleanup task dependency from drm_display_mode. It removes the use of drm_mode_object from drm_display_mode struct and it removes the use of base.id and base.type from drm_display_mode struct print string. Signed-off-by: Shayenne Moura Reviewed-by: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/f40e904e665fe3e3ae3ae86e837024bee3b8ca6d.1547214023.git.shayenneluzmoura@gmail.com --- include/drm/drm_modes.h | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) (limited to 'include') diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index baded6514456..be4fed97e727 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -136,8 +136,7 @@ enum drm_mode_status { .hdisplay = (hd), .hsync_start = (hss), .hsync_end = (hse), \ .htotal = (ht), .hskew = (hsk), .vdisplay = (vd), \ .vsync_start = (vss), .vsync_end = (vse), .vtotal = (vt), \ - .vscan = (vs), .flags = (f), \ - .base.type = DRM_MODE_OBJECT_MODE + .vscan = (vs), .flags = (f) #define CRTC_INTERLACE_HALVE_V (1 << 0) /* halve V values for interlacing */ #define CRTC_STEREO_DOUBLE (1 << 1) /* adjust timings for stereo modes */ @@ -213,20 +212,6 @@ struct drm_display_mode { */ struct list_head head; - /** - * @base: - * - * A display mode is a normal modeset object, possibly including public - * userspace id. - * - * FIXME: - * - * This can probably be removed since the entire concept of userspace - * managing modes explicitly has never landed in upstream kernel mode - * setting support. - */ - struct drm_mode_object base; - /** * @name: * @@ -429,14 +414,14 @@ struct drm_display_mode { /** * DRM_MODE_FMT - printf string for &struct drm_display_mode */ -#define DRM_MODE_FMT "%d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x" +#define DRM_MODE_FMT "\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x" /** * DRM_MODE_ARG - printf arguments for &struct drm_display_mode * @m: display mode */ #define DRM_MODE_ARG(m) \ - (m)->base.id, (m)->name, (m)->vrefresh, (m)->clock, \ + (m)->name, (m)->vrefresh, (m)->clock, \ (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \ (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \ (m)->type, (m)->flags -- cgit v1.2.3 From 94520db52fc0e931327bb77fe79a952a0e9dd2b0 Mon Sep 17 00:00:00 2001 From: Sam Ravnborg Date: Tue, 15 Jan 2019 22:48:45 +0100 Subject: drm: fix alpha build after drm_util.h change 0-DAY reported the following bug: tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next head: 21376e2c3c5bad5e87ba700c055c8a8235c2bfd5 commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h config: alpha-allmodconfig (attached as .config) ... In file included from include/linux/irqflags.h:16:0, from include/drm/drm_util.h:35, from drivers/gpu/drm/qxl/qxl_cmd.c:28: >> arch/alpha/include/asm/irqflags.h:58:15: error: unknown type name 'bool' static inline bool arch_irqs_disabled_flags(unsigned long flags) ^~~~ And later following bug: tree: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next head: 21376e2c3c5bad5e87ba700c055c8a8235c2bfd5 commit: e9eafcb589213395232084a2378e2e90f67feb29 [1/2] drm: move drm_can_sleep() to drm_util.h config: ia64-allyesconfig (attached as .config) ... In file included from arch/ia64/include/asm/irqflags.h:14, from include/linux/irqflags.h:16, from include/drm/drm_util.h:35, from drivers/gpu/drm/qxl/qxl_cmd.c:28: arch/ia64/include/asm/pal.h: In function 'ia64_pal_tr_read': arch/ia64/include/asm/pal.h:1703:64: error: implicit declaration of function 'ia64_tpa'; did you mean 'ia64_pal'? [-Werror=implicit-function-declaration] PAL_CALL_PHYS_STK(iprv, PAL_VM_TR_READ, reg_num, tr_type,(u64)ia64_tpa(tr_buffer)); ^~~~~~~~ ... So we have a situation where we do not pull in when building for alpha and for ia64 we need even more definitions are required. Two invasive fixes where considered: - Change all declarations of arch_irqs_disabled_flags() to use bool - Add include of to all files that uses bool for arch_irqs_disabled_flags To invasive with a too high pain/benefit ratio, so dropped. They would not cover ia64 either. Some less invasive fixes was also considered: - Add include of to drm_util.h - Add include of to drm_util.h The first was dropped as this did not cover the ia64 case. The latter was considered the best option as there could be other similar cases and we would like the header files below include/drm/ to be selfcontained. So we end up pulling in a lot of stuff not needed, but this is the price we pay in drm/ because the kernel headers are not all selfcontained. While at it, ordred the includefiles in drm_util in alphabetical order. Build tested with alpha,ia64,arm,x86 with allmodconfig and allyesconfig. v2: - fix ia64 build, changed to include interrupt.h - sort include files alphabetically Fixes: 733748ac37b45 ("drm: move drm_can_sleep() to drm_util.h") Signed-off-by: Sam Ravnborg Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20190115214845.8117-1-sam@ravnborg.org --- include/drm/drm_util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h index 0500da65b1d1..8163d35f8327 100644 --- a/include/drm/drm_util.h +++ b/include/drm/drm_util.h @@ -32,9 +32,9 @@ * Macros and inline functions that does not naturally belong in other places */ -#include -#include +#include #include +#include #include /* -- cgit v1.2.3