mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mahadevan P <mahadevan.p@oss.qualcomm.com>
To: Rob Clark <robin.clark@oss.qualcomm.com>,
	Sean Paul <sean@poorly.run>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Akhil P Oommen <akhilpo@oss.qualcomm.com>,
	Dmitry Baryshkov <lumag@kernel.org>,
	Abhinav Kumar <abhinav.kumar@linux.dev>,
	Jessica Zhang <jesszhan0024@gmail.com>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>
Cc: linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] drm/msm: default separate_gpu_kms to auto selection
Date: Thu, 30 Jul 2026 18:29:36 +0530	[thread overview]
Message-ID: <cd76a3aa-f86d-49be-a94b-820f4549d4dd@oss.qualcomm.com> (raw)
In-Reply-To: <7cb406ca-e631-4cd4-9782-d3efe30844e4@oss.qualcomm.com>

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 <mahadevan.p@oss.qualcomm.com>
>> ---
>> Depends on:
>>    https://lore.kernel.org/lkml/20260124-adreno-module-table- 
>> v1-1-9c2dbb2638b4@oss.qualcomm.com/
>>
>>    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- 
>> v2-1-b027ca97b9fe@oss.qualcomm.com
>>
>> 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- 
>> v1-1-e8231e7f1685@oss.qualcomm.com
>> ---
>>   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



      reply	other threads:[~2026-07-30 12:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-30  9:52 Mahadevan P
2026-04-13  8:44 ` Mahadevan P
2026-07-30 12:59   ` Mahadevan P [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cd76a3aa-f86d-49be-a94b-820f4549d4dd@oss.qualcomm.com \
    --to=mahadevan.p@oss.qualcomm.com \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=akhilpo@oss.qualcomm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=jesszhan0024@gmail.com \
    --cc=konradybcio@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®