Re: [PATCH v3] drm/msm: default separate_gpu_kms to auto selection
Mahadevan P <[email protected]>
| Newsgroups | org.freedesktop.lists.dri-devel,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Dmitry, Rob, Gentle reminder on this one . Happy to rework anything that doesn't look right; On 4/13/2026 2:14 PM, Mahadevan P wrote: > Hi all, > > Just a gentle ping on this patch below. > Any feedback would be very helpful. > > On 3/30/2026 3:22 PM, Mahadevan P wrote: >> On platforms with multiple display subsystems, such as SA8775P, the GPU >> binds to the first display subsystem that probes. This implicit binding >> prevents subsequent display subsystems from probing successfully, >> breaking multi-display support. >> >> Use the tristate separate_gpu_kms module parameter with the default >> value set to auto (-1). In auto mode, the driver selects the binding >> behavior based on the number of GPUs and display subsystems. This allows >> display subsystems to probe independently when required, while >> preserving the existing single-card behavior on simpler systems. >> >> The separate_gpu_kms module parameter has the following semantics: >> >> -1 (auto, default): >> Select the binding mode based on hardware topology. If exactly one >> GPU and one display subsystem are present, bind them together to >> form a single DRM device. Otherwise, expose the GPU and display >> subsystems as separate DRM devices. >> >> 0: >> Always bind the GPU and display together to form a single DRM >> device. >> >> 1: >> Always expose the GPU and display subsystems as separate DRM >> devices. >> >> Additionally, ensure that display subsystems are always exposed as >> separate DRM devices when no recognized Adreno GPU is present, >> regardless of the separate_gpu_kms setting. >> >> This ensures correct probing on multi-display platforms without >> affecting single-display, single-GPU systems. >> >> Signed-off-by: Mahadevan P <[email protected]> >> --- >> Depends on: >> https://lore.kernel.org/lkml/20260124-adreno-module-table- >> [email protected]/ >> >> When separate_gpu_kms is enabled, the GPU and display drivers are >> probed independently. In this configuration, the Adreno GPU driver >> may no longer be loaded implicitly via the display subsystem. >> >> The referenced patch adds a MODULE_DEVICE_TABLE() entry for the >> Adreno GPU device, ensuring proper module autoloading based on >> device tree matching. This is required to guarantee that the GPU >> driver is loaded correctly when GPU and display probing are >> decoupled. >> >> Changes in v3: >> - Ensure display subsystems are always exposed as separate DRM devices >> when >> no recognized Adreno GPU is present (!adreno_has_gpu()), regardless of >> separate_gpu_kms setting. (Dmitry) >> - Extend auto-selection logic to account for legacy display >> controllers (MDP4 and MDP5). (Dmitry) >> - Rename msm_mdss_count_masters() to msm_mdss_count() to better >> reflect functionality. (Dmitry) >> - Common helper to determine presence of a valid Adreno GPU node, >> avoiding repeated >> adreno_has_gpu() and availability checks. >> - Update commit message to reflect the above behavior. >> - Link to v2: https://lore.kernel.org/r/20260317-separate_gpu_kms- >> [email protected] >> >> Changes in v2: >> - Drop dependency on Lemans dual-DPU device tree changes as this patch >> works independently (Dmitry) >> - Switch separate_gpu_kms to tristate and default to auto mode (Rob) >> - Rename msm_gpu_no_components() to msm_separate_gpu_kms_components() >> for clarity >> - Link to v1: https://lore.kernel.org/r/20260223-seperate_gpu_kms- >> [email protected] >> --- >> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +- >> drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 14 +++++++ >> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 +++++++ >> drivers/gpu/drm/msm/msm_drv.c | 64 ++++++++++++++++++++ >> ++++++---- >> drivers/gpu/drm/msm/msm_drv.h | 8 +++- >> drivers/gpu/drm/msm/msm_mdss.c | 15 +++++++ >> 6 files changed, 107 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/ >> drm/msm/adreno/adreno_device.c >> index 4edfe80c5be7..e40648c05797 100644 >> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c >> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c >> @@ -272,7 +272,7 @@ static const struct component_ops a3xx_ops = { >> static int adreno_probe(struct platform_device *pdev) >> { >> if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon") || >> - msm_gpu_no_components()) >> + msm_separate_gpu_kms_components()) >> return msm_gpu_probe(pdev, &a3xx_ops); >> return component_add(&pdev->dev, &a3xx_ops); >> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/ >> drm/msm/disp/mdp4/mdp4_kms.c >> index 809ca191e9de..409000b739b6 100644 >> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c >> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c >> @@ -15,6 +15,20 @@ >> #include "msm_mmu.h" >> #include "mdp4_kms.h" >> +static const struct of_device_id mdp4_dt_match[]; >> +int msm_count_mdp4(void) >> +{ >> + struct device_node *np; >> + int count = 0; >> + >> + for_each_matching_node(np, mdp4_dt_match) { >> + if (of_device_is_available(np)) >> + count++; >> + } >> + >> + return count; >> +} >> + >> static int mdp4_hw_init(struct msm_kms *kms) >> { >> struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms)); >> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/ >> drm/msm/disp/mdp5/mdp5_kms.c >> index 1e3dc9bf9494..086895c9f103 100644 >> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c >> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c >> @@ -19,6 +19,20 @@ >> #include "msm_mmu.h" >> #include "mdp5_kms.h" >> +static const struct of_device_id mdp5_dt_match[]; >> +int msm_count_mdp5(void) >> +{ >> + struct device_node *np; >> + int count = 0; >> + >> + for_each_matching_node(np, mdp5_dt_match) { >> + if (of_device_is_available(np)) >> + count++; >> + } >> + >> + return count; >> +} >> + >> static int mdp5_hw_init(struct msm_kms *kms) >> { >> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms)); >> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/ >> msm_drv.c >> index e5ab1e28851d..e2101c6632ac 100644 >> --- a/drivers/gpu/drm/msm/msm_drv.c >> +++ b/drivers/gpu/drm/msm/msm_drv.c >> @@ -54,16 +54,64 @@ static bool modeset = true; >> MODULE_PARM_DESC(modeset, "Use kernel modesetting [KMS] (1=on >> (default), 0=disable)"); >> module_param(modeset, bool, 0600); >> -static bool separate_gpu_kms; >> -MODULE_PARM_DESC(separate_gpu_drm, "Use separate DRM device for the >> GPU (0=single DRM device for both GPU and display (default), 1=two DRM >> devices)"); >> -module_param(separate_gpu_kms, bool, 0400); >> +/* >> + * separate_gpu_kms (tristate): >> + * -1 (default): decide automatically based on hardware topology. >> Split devices >> + * if there is more than one GPU or more than one >> display master. >> + * 0: force single DRM device (bind display + GPU) >> + * 1: force separate DRM devices >> + */ >> +static int separate_gpu_kms = -1; >> +MODULE_PARM_DESC(separate_gpu_kms, >> + "Use separate DRM device for the GPU (-1=auto (default), >> 0=single DRM device, 1=separate DRM devices)"); >> +module_param(separate_gpu_kms, int, 0400); >> DECLARE_FAULT_ATTR(fail_gem_alloc); >> DECLARE_FAULT_ATTR(fail_gem_iova); >> -bool msm_gpu_no_components(void) >> +static const struct of_device_id msm_gpu_match[]; >> +static inline bool msm_gpu_node_present(struct device_node *np) >> +{ >> + return np && of_device_is_available(np) && adreno_has_gpu(np); >> +} >> + >> +static int msm_count_gpus(void) >> +{ >> + struct device_node *np; >> + int count = 0; >> + >> + for_each_matching_node(np, msm_gpu_match) { >> + if (msm_gpu_node_present(np)) >> + count++; >> + } >> + >> + return count; >> +} >> + >> +static bool msm_separate_gpu_kms_auto(void) >> +{ >> + int gpus = msm_count_gpus(); >> + int display_subsystems = msm_count_mdss() + msm_count_mdp4() + >> msm_count_mdp5(); >> + >> + if (gpus <= 0 || display_subsystems <= 0) >> + return false; >> + >> + /* If exactly one GPU and one display subsystem single card */ >> + return (gpus > 1) || (display_subsystems > 1); >> +} >> + >> +bool msm_separate_gpu_kms_components(void) >> { >> - return separate_gpu_kms; >> + struct device_node *np; >> + >> + np = of_find_matching_node(NULL, msm_gpu_match); >> + if (!msm_gpu_node_present(np)) >> + return true; >> + if (separate_gpu_kms == 1) >> + return true; >> + if (separate_gpu_kms == 0) >> + return false; >> + return msm_separate_gpu_kms_auto(); >> } >> static int msm_drm_uninit(struct device *dev, const struct >> component_ops *gpu_ops) >> @@ -1019,7 +1067,7 @@ static int add_gpu_components(struct device *dev, >> if (!np) >> return 0; >> - if (of_device_is_available(np) && adreno_has_gpu(np)) >> + if (msm_gpu_node_present(np)) >> drm_of_component_match_add(dev, matchptr, >> component_compare_of, np); >> of_node_put(np); >> @@ -1030,7 +1078,7 @@ static int add_gpu_components(struct device *dev, >> static int msm_drm_bind(struct device *dev) >> { >> return msm_drm_init(dev, >> - msm_gpu_no_components() ? >> + msm_separate_gpu_kms_components() ? >> &msm_kms_driver : >> &msm_driver, >> NULL); >> @@ -1069,7 +1117,7 @@ int msm_drv_probe(struct device *master_dev, >> return ret; >> } >> - if (!msm_gpu_no_components()) { >> + if (!msm_separate_gpu_kms_components()) { >> ret = add_gpu_components(master_dev, &match); >> if (ret) >> return ret; >> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/ >> msm_drv.h >> index 6d847d593f1a..a977fe7b36b8 100644 >> --- a/drivers/gpu/drm/msm/msm_drv.h >> +++ b/drivers/gpu/drm/msm/msm_drv.h >> @@ -555,6 +555,12 @@ void msm_kms_shutdown(struct platform_device *pdev); >> bool msm_disp_drv_should_bind(struct device *dev, bool dpu_driver); >> -bool msm_gpu_no_components(void); >> +bool msm_separate_gpu_kms_components(void); >> + >> +int msm_count_mdss(void); >> + >> +int msm_count_mdp4(void); >> + >> +int msm_count_mdp5(void); >> #endif /* __MSM_DRV_H__ */ >> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/ >> msm_mdss.c >> index 9047e8d9ee89..4c788f2647b0 100644 >> --- a/drivers/gpu/drm/msm/msm_mdss.c >> +++ b/drivers/gpu/drm/msm/msm_mdss.c >> @@ -73,6 +73,21 @@ static int msm_mdss_parse_data_bus_icc_path(struct >> device *dev, >> return 0; >> } >> +static const struct of_device_id mdss_dt_match[]; >> + >> +int msm_count_mdss(void) >> +{ >> + struct device_node *np; >> + int count = 0; >> + >> + for_each_matching_node(np, mdss_dt_match) { >> + if (of_device_is_available(np)) >> + count++; >> + } >> + >> + return count; >> +} >> + >> static void msm_mdss_irq(struct irq_desc *desc) >> { >> struct msm_mdss *msm_mdss = irq_desc_get_handler_data(desc); >> >> --- >> base-commit: b84a0ebe421ca56995ff78b66307667b62b3a900 >> change-id: 20260316-separate_gpu_kms-04d2cf4d91e2 >> >> Best regards, > Thanks, > Mahadevan Thanks, Mahadevan