[Deepin-Kernel-SIG] [linux 6.6-y] [Inspur] Add Inspur Graphics PCI-E DRM Driver （[6.6] [Feature] : inspur-drm驱动合入）#834
Merged
Conversation
Add Inspur DRM driver for Inspur BMC SoC. Link: https://gitee.com/anolis/cloud-kernel/pulls/3046 Signed-off-by: wangkaiyuan <wangkaiyuan@inspur.com> [ WangYuli: Edit the commit msg. ] Signed-off-by: WangYuli <wangyuli@uniontech.com>
Reviewer's GuideIntroduces a new Inspur DRM driver by updating build configurations and adding a set of driver sources under drivers/gpu/drm/inspur/inspur-drm, implementing PCI probe, memory mapping, power management, atomic plane/CRTC/encoder/connector support, dumb buffer creation, VRAM management, and detailed hardware register definitions. Sequence Diagram for Inspur DRM PCI Probe and LoadsequenceDiagram
participant PCI_Subsystem as Kernel (PCI Subsystem)
participant pci_driver as inspur_pci_driver_impl
participant DRM_Core as Kernel (DRM Core)
participant inspur_module as inspur_drm_module
PCI_Subsystem->>pci_driver: inspur_pci_probe(pdev, id)
pci_driver->>DRM_Core: drm_dev_alloc()
Note right of pci_driver: dev created, priv allocated
pci_driver->>PCI_Subsystem: pci_enable_device(pdev)
pci_driver->>inspur_module: inspur_load(dev, id->driver_data)
activate inspur_module
inspur_module->>inspur_module: inspur_hw_init(priv)
Note right of inspur_module: Maps MMIO & FB, configures HW
inspur_module->>DRM_Core: drmm_vram_helper_init(dev, fb_base, fb_size)
inspur_module->>inspur_module: inspur_kms_init(priv)
activate inspur_module
inspur_module->>inspur_module: inspur_de_init(priv) // Init display engine (CRTC, Plane)
inspur_module->>inspur_module: inspur_vdac_init(priv) // Init DAC (Encoder, Connector)
deactivate inspur_module
Note right of inspur_module: KMS components initialized
inspur_module->>DRM_Core: drm_mode_config_reset(dev)
deactivate inspur_module
pci_driver->>DRM_Core: drm_dev_register(dev, id->driver_data)
pci_driver->>DRM_Core: drm_fbdev_generic_setup(dev, depth)
Sequence Diagram for Inspur Plane Atomic UpdatesequenceDiagram
participant DRM_Core
participant PlaneHelpers as inspur_plane_helper_funcs
participant DriverPriv as inspur_drm_private_instance
participant HW as Inspur Graphics Hardware
DRM_Core->>PlaneHelpers: atomic_check(plane, atomic_state)
activate PlaneHelpers
PlaneHelpers->>PlaneHelpers: inspur_plane_atomic_check(plane, new_plane_state)
Note right of PlaneHelpers: Validate plane state (src/crtc size, position, pitch)
deactivate PlaneHelpers
DRM_Core->>PlaneHelpers: atomic_update(plane, old_atomic_state)
activate PlaneHelpers
PlaneHelpers->>PlaneHelpers: inspur_plane_atomic_update(plane, old_plane_state)
Note right of PlaneHelpers: Get framebuffer object (gbo)
PlaneHelpers->>DRM_Core: drm_gem_vram_pin(gbo)
PlaneHelpers->>DRM_Core: drm_gem_vram_offset(gbo)
Note right of PlaneHelpers: Get GPU address (gpu_addr)
PlaneHelpers->>DriverPriv: Access priv->mmio
DriverPriv->>HW: writel(gpu_addr, mmio + INSPUR_CRT_FB_ADDRESS)
DriverPriv->>HW: writel(fb_width_reg, mmio + INSPUR_CRT_FB_WIDTH)
DriverPriv->>HW: writel(disp_ctl_reg, mmio + INSPUR_CRT_DISP_CTL) // Set pixel format
deactivate PlaneHelpers
ER Diagram for Inspur DRM Driver Core Data StructureserDiagram
DRM_DEVICE {
string name
inspur_drm_private_ref dev_private
}
INSPUR_DRM_PRIVATE {
drm_device_ref dev
pointer mmio
pointer fb_map
inspur_fbdev_ref fbdev
inspur_cursor cursor
bool mode_config_initialized
drm_atomic_state_ref suspend_state
}
INSPUR_FBDEV {
drm_fb_helper helper
drm_framebuffer_ref fb
int size
}
INSPUR_CURSOR {
drm_gem_vram_object_array gbo
int next_index
}
INSPUR_DISPLAY_PLL_CONFIG {
unsigned_long hdisplay
unsigned_long vdisplay
u32 pll1_config_value
u32 pll2_config_value
}
DRM_DEVICE ||--o{ INSPUR_DRM_PRIVATE : "has (as dev_private)"
INSPUR_DRM_PRIVATE }o--|| DRM_DEVICE : "points to (dev)"
INSPUR_DRM_PRIVATE ||--|{ INSPUR_FBDEV : "aggregates"
INSPUR_DRM_PRIVATE ||--|{ INSPUR_CURSOR : "aggregates"
INSPUR_DRM_PRIVATE }o..o{ INSPUR_DISPLAY_PLL_CONFIG : "uses (indirectly for mode setting)"
Class Diagram for New Inspur DRM Driver Structures and CallbacksclassDiagram
class inspur_drm_private {
+void* mmio
+void* fb_map
+unsigned long fb_base
+unsigned long fb_size
+drm_device* dev
+bool mode_config_initialized
+drm_atomic_state* suspend_state
+inspur_fbdev* fbdev
+inspur_cursor cursor
+inspur_set_power_mode(priv, mode) void
+inspur_set_current_gate(priv, gate) void
+inspur_hw_init(priv) int
+inspur_kms_init(priv) int
+inspur_de_init(priv) int
+inspur_vdac_init(priv) int
}
class inspur_fbdev {
+drm_fb_helper helper
+drm_framebuffer* fb
+int size
}
class inspur_cursor {
+drm_gem_vram_object* gbo[2]
+unsigned int next_index
}
class inspur_dislay_pll_config {
<<Display PLL Configuration Data>>
+unsigned long hdisplay
+unsigned long vdisplay
+u32 pll1_config_value
+u32 pll2_config_value
}
inspur_drm_private "1" *-- "1" inspur_fbdev : owns
inspur_drm_private "1" *-- "1" inspur_cursor : owns
class inspur_pci_driver_callbacks {
<<Static Callbacks for PCI Driver>>
+inspur_pci_probe(pdev, ent) int
+inspur_pci_remove(pdev) void
+inspur_pci_shutdown(pdev) void
+inspur_pm_suspend(dev) int
+inspur_pm_resume(dev) int
}
class inspur_drm_driver_callbacks {
<<Static Callbacks for DRM Driver>>
+inspur_load(dev, flags) int
+inspur_unload(dev) void
+inspur_dumb_create(file, dev, args) int
+inspur_drm_interrupt(irq, arg) irqreturn_t
}
inspur_pci_driver_callbacks ..> inspur_drm_driver_callbacks : (probe invokes load)
inspur_drm_driver_callbacks ..> inspur_drm_private : (load creates & initializes private data)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a new Inspur DRM driver for Inspur BMC SoC to the Linux 6.6-y kernel, providing hardware initialization, mode configuration, and integration with DRM core frameworks. Key changes include:
- Implementation of dumb buffer creation in inspur_ttm.c.
- Initialization of encoder and connector in inspur_drm_vdac.c.
- Addition of register definitions, driver header files, and integration entries in Makefiles and Kconfig files.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| drivers/gpu/drm/inspur/inspur-drm/inspur_ttm.c | Adds a function for dumb buffer creation using DRM helpers. |
| drivers/gpu/drm/inspur/inspur-drm/inspur_drm_vdac.c | Implements encoder and connector initialization with error messages. |
| drivers/gpu/drm/inspur/inspur-drm/inspur_drm_regs.h | Provides register definitions for the Inspur DRM driver. |
| drivers/gpu/drm/inspur/inspur-drm/inspur_drm_drv.h | Declares driver and function prototypes for Inspur DRM. |
| drivers/gpu/drm/inspur/inspur-drm/Makefile | Lists source files for the Inspur DRM driver. |
| drivers/gpu/drm/inspur/inspur-drm/Kconfig | Configures module options for the Inspur DRM driver. |
| drivers/gpu/drm/inspur/Makefile | Top-level Makefile addition for the Inspur DRM driver. |
| drivers/gpu/drm/inspur/Kconfig | Integrates the Inspur DRM driver Kconfig into the DRM subsystem. |
| drivers/gpu/drm/Makefile, drivers/gpu/drm/Kconfig | Updated to include the Inspur DRM driver in the build system. |
| struct drm_mode_create_dumb *args) | ||
| { | ||
|
|
||
| return drm_gem_vram_fill_create_dumb(file, dev, 0, 16, args); |
There was a problem hiding this comment.
Consider adding a comment to explain the rationale behind using the magic numbers (0 and 16) in this helper call to improve code clarity.
|
|
||
| encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL); | ||
| if (!encoder) { | ||
| DRM_ERROR("failed to alloc memory when init encoder\n"); |
There was a problem hiding this comment.
Consider changing 'alloc' to 'allocate' for clearer error messaging.
Suggested change
| DRM_ERROR("failed to alloc memory when init encoder\n"); | |
| DRM_ERROR("failed to allocate memory when initializing encoder\n"); |
|
|
||
| connector = devm_kzalloc(dev->dev, sizeof(*connector), GFP_KERNEL); | ||
| if (!connector) { | ||
| DRM_ERROR("failed to alloc memory when init connector\n"); |
There was a problem hiding this comment.
Consider changing 'alloc' to 'allocate' in the error message to improve clarity.
Suggested change
| DRM_ERROR("failed to alloc memory when init connector\n"); | |
| DRM_ERROR("failed to allocate memory when init connector\n"); |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Avenger-285714 - I've reviewed your changes - here's some feedback:
- You never register your interrupt handler (via request_irq or pci_enable_msi), so inspur_drm_interrupt will never fire and free_irq in unload is unmatched—please add proper IRQ registration and teardown in inspur_load/inspur_unload.
- The inspur_pll_table only covers five common modes but regs.h defines PLL settings for additional resolutions (e.g. 1280×800, 1440×900, 1680×1050, 1920×1200); please expand the table or log a warning when falling back to the default PLL.
- The PADDING macro in inspur_drm_de.c is never used—consider removing it to clean up dead code.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| gbo = drm_gem_vram_of_gem(state->fb->obj[0]); | ||
|
|
||
| ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM); |
There was a problem hiding this comment.
issue (bug_risk): Pinned GEM object is not unpinned after use
Add a call to drm_gem_vram_unpin() after programming the registers to avoid leaking a pin reference.
| } | ||
|
|
||
| /* if found none, we use default value */ | ||
| *pll1 = CRT_PLL1_NS_25MHZ; |
There was a problem hiding this comment.
issue: Silent fallback to default PLL config
Please add a log message when defaulting to 25MHz to aid in diagnosing unsupported modes.
Member
Author
|
使用LLVM20编译工具链手工检查，使能DRM_INSPUR可通过编译。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add Inspur DRM driver for Inspur BMC SoC.
Link: https://gitee.com/anolis/cloud-kernel/pulls/3046
[ WangYuli: Edit the commit msg. ]
Summary by Sourcery
Add a complete Inspur DRM driver implementation including build integration, PCI device probing, display engine setup, memory management, interrupt handling, and power/clock control
New Features: