* [PATCH 1/7] drm/msm: move wq handling to KMS code
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
@ 2025-04-13 16:32 ` Dmitry Baryshkov
2025-04-14 15:58 ` Rob Clark
2025-04-13 16:32 ` [PATCH 2/7] drm/msm: move helper calls to msm_kms.c Dmitry Baryshkov
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:32 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
The global workqueue is only used for vblanks inside KMS code. Move
allocation / flushing / deallcation of it to msm_kms.c
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_drv.c | 21 ++-------------------
drivers/gpu/drm/msm/msm_kms.c | 16 +++++++++++++++-
2 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index c3588dc9e53764a27efda1901b094724cec8928a..02beb40eb9146941aa43862d07a6d82ae21c965e 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -82,13 +82,6 @@ static int msm_drm_uninit(struct device *dev)
drm_atomic_helper_shutdown(ddev);
}
- /* We must cancel and cleanup any pending vblank enable/disable
- * work before msm_irq_uninstall() to avoid work re-enabling an
- * irq after uninstall has disabled it.
- */
-
- flush_workqueue(priv->wq);
-
msm_gem_shrinker_cleanup(ddev);
msm_perf_debugfs_cleanup(priv);
@@ -104,8 +97,6 @@ static int msm_drm_uninit(struct device *dev)
ddev->dev_private = NULL;
drm_dev_put(ddev);
- destroy_workqueue(priv->wq);
-
return 0;
}
@@ -227,12 +218,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
ddev->dev_private = priv;
priv->dev = ddev;
- priv->wq = alloc_ordered_workqueue("msm", 0);
- if (!priv->wq) {
- ret = -ENOMEM;
- goto err_put_dev;
- }
-
INIT_LIST_HEAD(&priv->objects);
mutex_init(&priv->obj_lock);
@@ -253,12 +238,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
if (priv->kms_init) {
ret = drmm_mode_config_init(ddev);
if (ret)
- goto err_destroy_wq;
+ goto err_put_dev;
}
ret = msm_init_vram(ddev);
if (ret)
- goto err_destroy_wq;
+ goto err_put_dev;
dma_set_max_seg_size(dev, UINT_MAX);
@@ -304,8 +289,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
err_deinit_vram:
msm_deinit_vram(ddev);
-err_destroy_wq:
- destroy_workqueue(priv->wq);
err_put_dev:
drm_dev_put(ddev);
diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index 35d5397e73b4c5cb90b1770e8570277e782be7ec..821f0b9f968fc3d448e612bfae04639ceb770353 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -227,6 +227,13 @@ void msm_drm_kms_uninit(struct device *dev)
BUG_ON(!kms);
+ /* We must cancel and cleanup any pending vblank enable/disable
+ * work before msm_irq_uninstall() to avoid work re-enabling an
+ * irq after uninstall has disabled it.
+ */
+
+ flush_workqueue(priv->wq);
+
/* clean up event worker threads */
for (i = 0; i < priv->num_crtcs; i++) {
if (priv->event_thread[i].worker)
@@ -243,6 +250,8 @@ void msm_drm_kms_uninit(struct device *dev)
if (kms && kms->funcs)
kms->funcs->destroy(kms);
+
+ destroy_workqueue(priv->wq);
}
int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
@@ -258,10 +267,14 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
if (ret)
return ret;
+ priv->wq = alloc_ordered_workqueue("msm", 0);
+ if (!priv->wq)
+ return -ENOMEM;
+
ret = priv->kms_init(ddev);
if (ret) {
DRM_DEV_ERROR(dev, "failed to load kms\n");
- return ret;
+ goto err_msm_uninit;
}
/* Enable normalization of plane zpos */
@@ -319,6 +332,7 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
return 0;
err_msm_uninit:
+ destroy_workqueue(priv->wq);
return ret;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/7] drm/msm: move wq handling to KMS code
2025-04-13 16:32 ` [PATCH 1/7] drm/msm: move wq handling to KMS code Dmitry Baryshkov
@ 2025-04-14 15:58 ` Rob Clark
2025-04-15 8:59 ` Dmitry Baryshkov
0 siblings, 1 reply; 14+ messages in thread
From: Rob Clark @ 2025-04-14 15:58 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Abhinav Kumar, Dmitry Baryshkov, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Konrad Dybcio, linux-arm-msm,
dri-devel, freedreno, linux-kernel
On Sun, Apr 13, 2025 at 9:33 AM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> The global workqueue is only used for vblanks inside KMS code. Move
> allocation / flushing / deallcation of it to msm_kms.c
Maybe we should also just move the wq into struct msm_kms?
BR,
-R
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 21 ++-------------------
> drivers/gpu/drm/msm/msm_kms.c | 16 +++++++++++++++-
> 2 files changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index c3588dc9e53764a27efda1901b094724cec8928a..02beb40eb9146941aa43862d07a6d82ae21c965e 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -82,13 +82,6 @@ static int msm_drm_uninit(struct device *dev)
> drm_atomic_helper_shutdown(ddev);
> }
>
> - /* We must cancel and cleanup any pending vblank enable/disable
> - * work before msm_irq_uninstall() to avoid work re-enabling an
> - * irq after uninstall has disabled it.
> - */
> -
> - flush_workqueue(priv->wq);
> -
> msm_gem_shrinker_cleanup(ddev);
>
> msm_perf_debugfs_cleanup(priv);
> @@ -104,8 +97,6 @@ static int msm_drm_uninit(struct device *dev)
> ddev->dev_private = NULL;
> drm_dev_put(ddev);
>
> - destroy_workqueue(priv->wq);
> -
> return 0;
> }
>
> @@ -227,12 +218,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
> ddev->dev_private = priv;
> priv->dev = ddev;
>
> - priv->wq = alloc_ordered_workqueue("msm", 0);
> - if (!priv->wq) {
> - ret = -ENOMEM;
> - goto err_put_dev;
> - }
> -
> INIT_LIST_HEAD(&priv->objects);
> mutex_init(&priv->obj_lock);
>
> @@ -253,12 +238,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
> if (priv->kms_init) {
> ret = drmm_mode_config_init(ddev);
> if (ret)
> - goto err_destroy_wq;
> + goto err_put_dev;
> }
>
> ret = msm_init_vram(ddev);
> if (ret)
> - goto err_destroy_wq;
> + goto err_put_dev;
>
> dma_set_max_seg_size(dev, UINT_MAX);
>
> @@ -304,8 +289,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>
> err_deinit_vram:
> msm_deinit_vram(ddev);
> -err_destroy_wq:
> - destroy_workqueue(priv->wq);
> err_put_dev:
> drm_dev_put(ddev);
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index 35d5397e73b4c5cb90b1770e8570277e782be7ec..821f0b9f968fc3d448e612bfae04639ceb770353 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -227,6 +227,13 @@ void msm_drm_kms_uninit(struct device *dev)
>
> BUG_ON(!kms);
>
> + /* We must cancel and cleanup any pending vblank enable/disable
> + * work before msm_irq_uninstall() to avoid work re-enabling an
> + * irq after uninstall has disabled it.
> + */
> +
> + flush_workqueue(priv->wq);
> +
> /* clean up event worker threads */
> for (i = 0; i < priv->num_crtcs; i++) {
> if (priv->event_thread[i].worker)
> @@ -243,6 +250,8 @@ void msm_drm_kms_uninit(struct device *dev)
>
> if (kms && kms->funcs)
> kms->funcs->destroy(kms);
> +
> + destroy_workqueue(priv->wq);
> }
>
> int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
> @@ -258,10 +267,14 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
> if (ret)
> return ret;
>
> + priv->wq = alloc_ordered_workqueue("msm", 0);
> + if (!priv->wq)
> + return -ENOMEM;
> +
> ret = priv->kms_init(ddev);
> if (ret) {
> DRM_DEV_ERROR(dev, "failed to load kms\n");
> - return ret;
> + goto err_msm_uninit;
> }
>
> /* Enable normalization of plane zpos */
> @@ -319,6 +332,7 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
> return 0;
>
> err_msm_uninit:
> + destroy_workqueue(priv->wq);
> return ret;
> }
>
>
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/7] drm/msm: move wq handling to KMS code
2025-04-14 15:58 ` Rob Clark
@ 2025-04-15 8:59 ` Dmitry Baryshkov
2025-04-30 19:41 ` Abhinav Kumar
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-15 8:59 UTC (permalink / raw)
To: Rob Clark
Cc: Abhinav Kumar, Dmitry Baryshkov, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Konrad Dybcio, linux-arm-msm,
dri-devel, freedreno, linux-kernel
On 14/04/2025 18:58, Rob Clark wrote:
> On Sun, Apr 13, 2025 at 9:33 AM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
>>
>> The global workqueue is only used for vblanks inside KMS code. Move
>> allocation / flushing / deallcation of it to msm_kms.c
>
> Maybe we should also just move the wq into struct msm_kms?
... together with several other KMS-only fields. I will take a look.
>
> BR,
> -R
>
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>> drivers/gpu/drm/msm/msm_drv.c | 21 ++-------------------
>> drivers/gpu/drm/msm/msm_kms.c | 16 +++++++++++++++-
>> 2 files changed, 17 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
>> index c3588dc9e53764a27efda1901b094724cec8928a..02beb40eb9146941aa43862d07a6d82ae21c965e 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.c
>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>> @@ -82,13 +82,6 @@ static int msm_drm_uninit(struct device *dev)
>> drm_atomic_helper_shutdown(ddev);
>> }
>>
>> - /* We must cancel and cleanup any pending vblank enable/disable
>> - * work before msm_irq_uninstall() to avoid work re-enabling an
>> - * irq after uninstall has disabled it.
>> - */
>> -
>> - flush_workqueue(priv->wq);
>> -
>> msm_gem_shrinker_cleanup(ddev);
>>
>> msm_perf_debugfs_cleanup(priv);
>> @@ -104,8 +97,6 @@ static int msm_drm_uninit(struct device *dev)
>> ddev->dev_private = NULL;
>> drm_dev_put(ddev);
>>
>> - destroy_workqueue(priv->wq);
>> -
>> return 0;
>> }
>>
>> @@ -227,12 +218,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>> ddev->dev_private = priv;
>> priv->dev = ddev;
>>
>> - priv->wq = alloc_ordered_workqueue("msm", 0);
>> - if (!priv->wq) {
>> - ret = -ENOMEM;
>> - goto err_put_dev;
>> - }
>> -
>> INIT_LIST_HEAD(&priv->objects);
>> mutex_init(&priv->obj_lock);
>>
>> @@ -253,12 +238,12 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>> if (priv->kms_init) {
>> ret = drmm_mode_config_init(ddev);
>> if (ret)
>> - goto err_destroy_wq;
>> + goto err_put_dev;
>> }
>>
>> ret = msm_init_vram(ddev);
>> if (ret)
>> - goto err_destroy_wq;
>> + goto err_put_dev;
>>
>> dma_set_max_seg_size(dev, UINT_MAX);
>>
>> @@ -304,8 +289,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
>>
>> err_deinit_vram:
>> msm_deinit_vram(ddev);
>> -err_destroy_wq:
>> - destroy_workqueue(priv->wq);
>> err_put_dev:
>> drm_dev_put(ddev);
>>
>> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
>> index 35d5397e73b4c5cb90b1770e8570277e782be7ec..821f0b9f968fc3d448e612bfae04639ceb770353 100644
>> --- a/drivers/gpu/drm/msm/msm_kms.c
>> +++ b/drivers/gpu/drm/msm/msm_kms.c
>> @@ -227,6 +227,13 @@ void msm_drm_kms_uninit(struct device *dev)
>>
>> BUG_ON(!kms);
>>
>> + /* We must cancel and cleanup any pending vblank enable/disable
>> + * work before msm_irq_uninstall() to avoid work re-enabling an
>> + * irq after uninstall has disabled it.
>> + */
>> +
>> + flush_workqueue(priv->wq);
>> +
>> /* clean up event worker threads */
>> for (i = 0; i < priv->num_crtcs; i++) {
>> if (priv->event_thread[i].worker)
>> @@ -243,6 +250,8 @@ void msm_drm_kms_uninit(struct device *dev)
>>
>> if (kms && kms->funcs)
>> kms->funcs->destroy(kms);
>> +
>> + destroy_workqueue(priv->wq);
>> }
>>
>> int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
>> @@ -258,10 +267,14 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
>> if (ret)
>> return ret;
>>
>> + priv->wq = alloc_ordered_workqueue("msm", 0);
>> + if (!priv->wq)
>> + return -ENOMEM;
>> +
>> ret = priv->kms_init(ddev);
>> if (ret) {
>> DRM_DEV_ERROR(dev, "failed to load kms\n");
>> - return ret;
>> + goto err_msm_uninit;
>> }
>>
>> /* Enable normalization of plane zpos */
>> @@ -319,6 +332,7 @@ int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
>> return 0;
>>
>> err_msm_uninit:
>> + destroy_workqueue(priv->wq);
>> return ret;
>> }
>>
>>
>> --
>> 2.39.5
>>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/7] drm/msm: move wq handling to KMS code
2025-04-15 8:59 ` Dmitry Baryshkov
@ 2025-04-30 19:41 ` Abhinav Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2025-04-30 19:41 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark
Cc: Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Konrad Dybcio, linux-arm-msm, dri-devel, freedreno,
linux-kernel
On 4/15/2025 1:59 AM, Dmitry Baryshkov wrote:
> On 14/04/2025 18:58, Rob Clark wrote:
>> On Sun, Apr 13, 2025 at 9:33 AM Dmitry Baryshkov
>> <dmitry.baryshkov@oss.qualcomm.com> wrote:
>>>
>>> The global workqueue is only used for vblanks inside KMS code. Move
>>> allocation / flushing / deallcation of it to msm_kms.c
>>
>> Maybe we should also just move the wq into struct msm_kms?
>
> ... together with several other KMS-only fields. I will take a look.
>
Yeah the usages seem to be only within kms, so we can move this to msm_kms.
>>
>> BR,
>> -R
>>
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>> ---
>>> drivers/gpu/drm/msm/msm_drv.c | 21 ++-------------------
>>> drivers/gpu/drm/msm/msm_kms.c | 16 +++++++++++++++-
>>> 2 files changed, 17 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/
>>> msm_drv.c
>>> index
>>> c3588dc9e53764a27efda1901b094724cec8928a..02beb40eb9146941aa43862d07a6d82ae21c965e 100644
>>> --- a/drivers/gpu/drm/msm/msm_drv.c
>>> +++ b/drivers/gpu/drm/msm/msm_drv.c
>>> @@ -82,13 +82,6 @@ static int msm_drm_uninit(struct device *dev)
>>> drm_atomic_helper_shutdown(ddev);
>>> }
>>>
>>> - /* We must cancel and cleanup any pending vblank enable/disable
>>> - * work before msm_irq_uninstall() to avoid work re-enabling an
>>> - * irq after uninstall has disabled it.
>>> - */
>>> -
>>> - flush_workqueue(priv->wq);
>>> -
>>> msm_gem_shrinker_cleanup(ddev);
>>>
>>> msm_perf_debugfs_cleanup(priv);
>>> @@ -104,8 +97,6 @@ static int msm_drm_uninit(struct device *dev)
>>> ddev->dev_private = NULL;
>>> drm_dev_put(ddev);
>>>
>>> - destroy_workqueue(priv->wq);
>>> -
>>> return 0;
>>> }
>>>
>>> @@ -227,12 +218,6 @@ static int msm_drm_init(struct device *dev,
>>> const struct drm_driver *drv)
>>> ddev->dev_private = priv;
>>> priv->dev = ddev;
>>>
>>> - priv->wq = alloc_ordered_workqueue("msm", 0);
>>> - if (!priv->wq) {
>>> - ret = -ENOMEM;
>>> - goto err_put_dev;
>>> - }
>>> -
>>> INIT_LIST_HEAD(&priv->objects);
>>> mutex_init(&priv->obj_lock);
>>>
>>> @@ -253,12 +238,12 @@ static int msm_drm_init(struct device *dev,
>>> const struct drm_driver *drv)
>>> if (priv->kms_init) {
>>> ret = drmm_mode_config_init(ddev);
>>> if (ret)
>>> - goto err_destroy_wq;
>>> + goto err_put_dev;
>>> }
>>>
>>> ret = msm_init_vram(ddev);
>>> if (ret)
>>> - goto err_destroy_wq;
>>> + goto err_put_dev;
>>>
>>> dma_set_max_seg_size(dev, UINT_MAX);
>>>
>>> @@ -304,8 +289,6 @@ static int msm_drm_init(struct device *dev, const
>>> struct drm_driver *drv)
>>>
>>> err_deinit_vram:
>>> msm_deinit_vram(ddev);
>>> -err_destroy_wq:
>>> - destroy_workqueue(priv->wq);
>>> err_put_dev:
>>> drm_dev_put(ddev);
>>>
>>> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/
>>> msm_kms.c
>>> index
>>> 35d5397e73b4c5cb90b1770e8570277e782be7ec..821f0b9f968fc3d448e612bfae04639ceb770353 100644
>>> --- a/drivers/gpu/drm/msm/msm_kms.c
>>> +++ b/drivers/gpu/drm/msm/msm_kms.c
>>> @@ -227,6 +227,13 @@ void msm_drm_kms_uninit(struct device *dev)
>>>
>>> BUG_ON(!kms);
>>>
>>> + /* We must cancel and cleanup any pending vblank enable/disable
>>> + * work before msm_irq_uninstall() to avoid work re-enabling an
>>> + * irq after uninstall has disabled it.
>>> + */
>>> +
>>> + flush_workqueue(priv->wq);
>>> +
>>> /* clean up event worker threads */
>>> for (i = 0; i < priv->num_crtcs; i++) {
>>> if (priv->event_thread[i].worker)
>>> @@ -243,6 +250,8 @@ void msm_drm_kms_uninit(struct device *dev)
>>>
>>> if (kms && kms->funcs)
>>> kms->funcs->destroy(kms);
>>> +
>>> + destroy_workqueue(priv->wq);
>>> }
>>>
>>> int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
>>> @@ -258,10 +267,14 @@ int msm_drm_kms_init(struct device *dev, const
>>> struct drm_driver *drv)
>>> if (ret)
>>> return ret;
>>>
>>> + priv->wq = alloc_ordered_workqueue("msm", 0);
>>> + if (!priv->wq)
>>> + return -ENOMEM;
>>> +
>>> ret = priv->kms_init(ddev);
>>> if (ret) {
>>> DRM_DEV_ERROR(dev, "failed to load kms\n");
>>> - return ret;
>>> + goto err_msm_uninit;
>>> }
>>>
>>> /* Enable normalization of plane zpos */
>>> @@ -319,6 +332,7 @@ int msm_drm_kms_init(struct device *dev, const
>>> struct drm_driver *drv)
>>> return 0;
>>>
>>> err_msm_uninit:
>>> + destroy_workqueue(priv->wq);
>>> return ret;
>>> }
>>>
>>>
>>> --
>>> 2.39.5
>>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/7] drm/msm: move helper calls to msm_kms.c
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
2025-04-13 16:32 ` [PATCH 1/7] drm/msm: move wq handling to KMS code Dmitry Baryshkov
@ 2025-04-13 16:32 ` Dmitry Baryshkov
2025-04-30 19:54 ` Abhinav Kumar
2025-04-13 16:32 ` [PATCH 3/7] drm/msm: make it possible to disable KMS-related code Dmitry Baryshkov
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:32 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Extract two more KMS-related codepieces to msm_kms.c, removing last
pieces of KMS code from msm_drv.c.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_drv.c | 9 +++------
drivers/gpu/drm/msm/msm_kms.c | 20 ++++++++++++++++++++
drivers/gpu/drm/msm/msm_kms.h | 2 ++
3 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 02beb40eb9146941aa43862d07a6d82ae21c965e..b296d1315aa249638e073d08d43e8d41a4354f3b 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -11,7 +11,6 @@
#include <linux/of_address.h>
#include <linux/uaccess.h>
-#include <drm/clients/drm_client_setup.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
#include <drm/drm_ioctl.h>
@@ -79,7 +78,7 @@ static int msm_drm_uninit(struct device *dev)
if (ddev->registered) {
drm_dev_unregister(ddev);
if (priv->kms)
- drm_atomic_helper_shutdown(ddev);
+ msm_drm_kms_unregister(dev);
}
msm_gem_shrinker_cleanup(ddev);
@@ -275,10 +274,8 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
if (ret)
goto err_msm_uninit;
- if (priv->kms_init) {
- drm_kms_helper_poll_init(ddev);
- drm_client_setup(ddev, NULL);
- }
+ if (priv->kms_init)
+ msm_drm_kms_post_init(dev);
return 0;
diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index 821f0b9f968fc3d448e612bfae04639ceb770353..fdb22c41bdc1a772b45c6940dda787dd96879bc2 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -13,6 +13,7 @@
#include <drm/drm_drv.h>
#include <drm/drm_mode_config.h>
#include <drm/drm_vblank.h>
+#include <drm/clients/drm_client_setup.h>
#include "disp/msm_disp_snapshot.h"
#include "msm_drv.h"
@@ -217,6 +218,15 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct drm_device *dev)
return aspace;
}
+void msm_drm_kms_unregister(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct msm_drm_private *priv = platform_get_drvdata(pdev);
+ struct drm_device *ddev = priv->dev;
+
+ drm_atomic_helper_shutdown(ddev);
+}
+
void msm_drm_kms_uninit(struct device *dev)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -373,3 +383,13 @@ void msm_kms_shutdown(struct platform_device *pdev)
if (drm && drm->registered && priv->kms)
drm_atomic_helper_shutdown(drm);
}
+
+void msm_drm_kms_post_init(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct msm_drm_private *priv = platform_get_drvdata(pdev);
+ struct drm_device *ddev = priv->dev;
+
+ drm_kms_helper_poll_init(ddev);
+ drm_client_setup(ddev, NULL);
+}
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 43b58d052ee6aae0ce34d09c88e1e1c34f9c52ef..057b20367724a4ca60f2d60c038077dbcc1c7abc 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -192,6 +192,8 @@ static inline void msm_kms_destroy(struct msm_kms *kms)
for_each_if (drm_crtc_mask(crtc) & (crtc_mask))
int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv);
+void msm_drm_kms_post_init(struct device *dev);
+void msm_drm_kms_unregister(struct device *dev);
void msm_drm_kms_uninit(struct device *dev);
#endif /* __MSM_KMS_H__ */
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/7] drm/msm: move helper calls to msm_kms.c
2025-04-13 16:32 ` [PATCH 2/7] drm/msm: move helper calls to msm_kms.c Dmitry Baryshkov
@ 2025-04-30 19:54 ` Abhinav Kumar
0 siblings, 0 replies; 14+ messages in thread
From: Abhinav Kumar @ 2025-04-30 19:54 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 4/13/2025 9:32 AM, Dmitry Baryshkov wrote:
> Extract two more KMS-related codepieces to msm_kms.c, removing last
> pieces of KMS code from msm_drv.c.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/msm_drv.c | 9 +++------
> drivers/gpu/drm/msm/msm_kms.c | 20 ++++++++++++++++++++
> drivers/gpu/drm/msm/msm_kms.h | 2 ++
> 3 files changed, 25 insertions(+), 6 deletions(-)
>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/7] drm/msm: make it possible to disable KMS-related code.
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
2025-04-13 16:32 ` [PATCH 1/7] drm/msm: move wq handling to KMS code Dmitry Baryshkov
2025-04-13 16:32 ` [PATCH 2/7] drm/msm: move helper calls to msm_kms.c Dmitry Baryshkov
@ 2025-04-13 16:32 ` Dmitry Baryshkov
2025-04-30 20:09 ` Abhinav Kumar
2025-04-13 16:32 ` [PATCH 4/7] drm/msm: bail out late_init_minor() if it is not a GPU device Dmitry Baryshkov
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:32 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
If the Adreno device is used in a headless mode, there is no need to
build all KMS components. Build corresponding parts conditionally, only
selecting them if modeset support is actually required.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/Kconfig | 14 ++++++
drivers/gpu/drm/msm/Makefile | 16 +++----
drivers/gpu/drm/msm/dp/dp_debug.c | 4 ++
drivers/gpu/drm/msm/msm_debugfs.c | 92 ++++++++++++++++++++++-----------------
drivers/gpu/drm/msm/msm_drv.h | 7 ++-
drivers/gpu/drm/msm/msm_kms.h | 23 ++++++++++
6 files changed, 108 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 974bc7c0ea761147d3326bdce9039d6f26f290d0..0be31c5acdd1b7ef80f0e820ff430692616122e0 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -66,6 +66,14 @@ config DRM_MSM_VALIDATE_XML
Validate XML files with register definitions against rules-fd schema.
This option is mostly targeting DRM MSM developers. If unsure, say N.
+config DRM_MSM_KMS
+ def_bool n
+ depends on DRM_MSM
+
+config DRM_MSM_KMS_FBDEV
+ def_bool DRM_FBDEV_EMULATION
+ depends on DRM_MSM_KMS
+
config DRM_MSM_MDSS
bool
depends on DRM_MSM
@@ -74,6 +82,7 @@ config DRM_MSM_MDSS
config DRM_MSM_MDP4
bool "Enable MDP4 support in MSM DRM driver"
depends on DRM_MSM
+ select DRM_MSM_KMS
default y
help
Compile in support for the Mobile Display Processor v4 (MDP4) in
@@ -84,6 +93,7 @@ config DRM_MSM_MDP5
bool "Enable MDP5 support in MSM DRM driver"
depends on DRM_MSM
select DRM_MSM_MDSS
+ select DRM_MSM_KMS
default y
help
Compile in support for the Mobile Display Processor v5 (MDP5) in
@@ -94,6 +104,7 @@ config DRM_MSM_DPU
bool "Enable DPU support in MSM DRM driver"
depends on DRM_MSM
select DRM_MSM_MDSS
+ select DRM_MSM_KMS
select DRM_DISPLAY_DSC_HELPER
default y
help
@@ -104,6 +115,7 @@ config DRM_MSM_DPU
config DRM_MSM_DP
bool "Enable DisplayPort support in MSM DRM driver"
depends on DRM_MSM
+ depends on DRM_MSM_KMS
select RATIONAL
default y
help
@@ -114,6 +126,7 @@ config DRM_MSM_DP
config DRM_MSM_DSI
bool "Enable DSI support in MSM DRM driver"
depends on DRM_MSM
+ depends on DRM_MSM_KMS
select DRM_PANEL
select DRM_MIPI_DSI
select DRM_DISPLAY_DSC_HELPER
@@ -169,6 +182,7 @@ config DRM_MSM_DSI_7NM_PHY
config DRM_MSM_HDMI
bool "Enable HDMI support in MSM DRM driver"
depends on DRM_MSM
+ depends on DRM_MSM_KMS
default y
select DRM_DISPLAY_HDMI_HELPER
select DRM_DISPLAY_HDMI_STATE_HELPER
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 5df20cbeafb8bf07c825a1fd72719d5a56c38613..3749b0af779e73a93d7c219d3fbd3865b9296b50 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -101,18 +101,15 @@ msm-display-$(CONFIG_DRM_MSM_DPU) += \
msm-display-$(CONFIG_DRM_MSM_MDSS) += \
msm_mdss.o \
-msm-display-y += \
+msm-display-$(CONFIG_DRM_MSM_KMS) += \
disp/mdp_format.o \
disp/mdp_kms.o \
disp/msm_disp_snapshot.o \
disp/msm_disp_snapshot_util.o \
msm-y += \
- msm_atomic.o \
- msm_atomic_tracepoints.o \
msm_debugfs.o \
msm_drv.o \
- msm_fb.o \
msm_fence.o \
msm_gem.o \
msm_gem_prime.o \
@@ -123,21 +120,24 @@ msm-y += \
msm_gpu_devfreq.o \
msm_io_utils.o \
msm_iommu.o \
- msm_kms.o \
msm_perf.o \
msm_rd.o \
msm_ringbuffer.o \
msm_submitqueue.o \
msm_gpu_tracepoints.o \
-msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
+msm-$(CONFIG_DRM_MSM_KMS) += \
+ msm_atomic.o \
+ msm_atomic_tracepoints.o \
+ msm_fb.o \
+ msm_kms.o \
-msm-display-$(CONFIG_DEBUG_FS) += \
- dp/dp_debug.o
+msm-$(CONFIG_DRM_MSM_KMS_FBDEV) += msm_fbdev.o
msm-display-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
dp/dp_catalog.o \
dp/dp_ctrl.o \
+ dp/dp_debug.o \
dp/dp_display.o \
dp/dp_drm.o \
dp/dp_link.o \
diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c
index 22fd946ee201397b72b43c8499714139deb7ba82..7835d93015b31a69aac824ca04dc65b374546e5c 100644
--- a/drivers/gpu/drm/msm/dp/dp_debug.c
+++ b/drivers/gpu/drm/msm/dp/dp_debug.c
@@ -5,6 +5,8 @@
#define pr_fmt(fmt)"[drm-dp] %s: " fmt, __func__
+#ifdef CONFIG_DEBUG_FS
+
#include <linux/debugfs.h>
#include <drm/drm_connector.h>
#include <drm/drm_file.h>
@@ -235,3 +237,5 @@ int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
return 0;
}
+
+#endif
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index 7ab607252d183f78b99c3a8b878c949ed5f99fec..2b12f2851fadbc3c924827e11570352736869614 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -117,6 +117,36 @@ static const struct file_operations msm_gpu_fops = {
.release = msm_gpu_release,
};
+#ifdef CONFIG_DRM_MSM_KMS
+static int msm_fb_show(struct seq_file *m, void *arg)
+{
+ struct drm_info_node *node = m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct drm_framebuffer *fb, *fbdev_fb = NULL;
+
+ if (dev->fb_helper && dev->fb_helper->fb) {
+ seq_puts(m, "fbcon ");
+ fbdev_fb = dev->fb_helper->fb;
+ msm_framebuffer_describe(fbdev_fb, m);
+ }
+
+ mutex_lock(&dev->mode_config.fb_lock);
+ list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
+ if (fb == fbdev_fb)
+ continue;
+
+ seq_puts(m, "user ");
+ msm_framebuffer_describe(fb, m);
+ }
+ mutex_unlock(&dev->mode_config.fb_lock);
+
+ return 0;
+}
+
+static struct drm_info_list msm_kms_debugfs_list[] = {
+ { "fb", msm_fb_show },
+};
+
/*
* Display Snapshot:
*/
@@ -180,6 +210,27 @@ static const struct file_operations msm_kms_fops = {
.release = msm_kms_release,
};
+static void msm_debugfs_kms_init(struct drm_minor *minor)
+{
+ struct drm_device *dev = minor->dev;
+ struct msm_drm_private *priv = dev->dev_private;
+
+ drm_debugfs_create_files(msm_kms_debugfs_list,
+ ARRAY_SIZE(msm_kms_debugfs_list),
+ minor->debugfs_root, minor);
+ debugfs_create_file("kms", 0400, minor->debugfs_root,
+ dev, &msm_kms_fops);
+
+ if (priv->kms->funcs->debugfs_init)
+ priv->kms->funcs->debugfs_init(priv->kms, minor);
+
+}
+#else /* ! CONFIG_DRM_MSM_KMS */
+static void msm_debugfs_kms_init(struct drm_minor *minor)
+{
+}
+#endif
+
/*
* Other debugfs:
*/
@@ -238,40 +289,11 @@ static int msm_mm_show(struct seq_file *m, void *arg)
return 0;
}
-static int msm_fb_show(struct seq_file *m, void *arg)
-{
- struct drm_info_node *node = m->private;
- struct drm_device *dev = node->minor->dev;
- struct drm_framebuffer *fb, *fbdev_fb = NULL;
-
- if (dev->fb_helper && dev->fb_helper->fb) {
- seq_printf(m, "fbcon ");
- fbdev_fb = dev->fb_helper->fb;
- msm_framebuffer_describe(fbdev_fb, m);
- }
-
- mutex_lock(&dev->mode_config.fb_lock);
- list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
- if (fb == fbdev_fb)
- continue;
-
- seq_printf(m, "user ");
- msm_framebuffer_describe(fb, m);
- }
- mutex_unlock(&dev->mode_config.fb_lock);
-
- return 0;
-}
-
static struct drm_info_list msm_debugfs_list[] = {
{"gem", msm_gem_show},
{ "mm", msm_mm_show },
};
-static struct drm_info_list msm_kms_debugfs_list[] = {
- { "fb", msm_fb_show },
-};
-
static int late_init_minor(struct drm_minor *minor)
{
int ret;
@@ -343,20 +365,12 @@ void msm_debugfs_init(struct drm_minor *minor)
if (priv->gpu_pdev)
msm_debugfs_gpu_init(minor);
- if (priv->kms) {
- drm_debugfs_create_files(msm_kms_debugfs_list,
- ARRAY_SIZE(msm_kms_debugfs_list),
- minor->debugfs_root, minor);
- debugfs_create_file("kms", S_IRUSR, minor->debugfs_root,
- dev, &msm_kms_fops);
- }
+ if (priv->kms)
+ msm_debugfs_kms_init(minor);
debugfs_create_file("shrink", S_IRWXU, minor->debugfs_root,
dev, &shrink_fops);
- if (priv->kms && priv->kms->funcs->debugfs_init)
- priv->kms->funcs->debugfs_init(priv->kms, minor);
-
fault_create_debugfs_attr("fail_gem_alloc", minor->debugfs_root,
&fail_gem_alloc);
fault_create_debugfs_attr("fail_gem_iova", minor->debugfs_root,
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index a65077855201746c37ee742364b61116565f3794..5f4d3f050c1fde71c405a1ebf516f4f5a396cfc4 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -88,6 +88,7 @@ struct msm_drm_private {
/* subordinate devices, if present: */
struct platform_device *gpu_pdev;
+#ifdef CONFIG_DRM_MSM_KMS
/* possibly this should be in the kms component, but it is
* shared by both mdp4 and mdp5..
*/
@@ -97,6 +98,7 @@ struct msm_drm_private {
struct msm_dsi *dsi[MSM_DSI_CONTROLLER_COUNT];
struct msm_dp *dp[MSM_DP_CONTROLLER_COUNT];
+#endif
/* when we have more than one 'msm_gpu' these need to be an array: */
struct msm_gpu *gpu;
@@ -177,11 +179,13 @@ struct msm_drm_private {
struct mutex lock;
} lru;
+#ifdef CONFIG_DRM_MSM_KMS
struct workqueue_struct *wq;
unsigned int num_crtcs;
struct msm_drm_thread event_thread[MAX_CRTCS];
+#endif
/* VRAM carveout, used when no IOMMU: */
struct {
@@ -275,7 +279,7 @@ struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev,
struct drm_framebuffer * msm_alloc_stolen_fb(struct drm_device *dev,
int w, int h, int p, uint32_t format);
-#ifdef CONFIG_DRM_FBDEV_EMULATION
+#ifdef CONFIG_DRM_MSM_KMS_FBDEV
int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes);
#define MSM_FBDEV_DRIVER_OPS \
@@ -360,6 +364,7 @@ static inline const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
}
#endif
+struct msm_dp;
#ifdef CONFIG_DRM_MSM_DP
int __init msm_dp_register(void);
void __exit msm_dp_unregister(void);
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 057b20367724a4ca60f2d60c038077dbcc1c7abc..aabbe851f0d533cca4a2ca00b79fbf8603870be3 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -13,6 +13,8 @@
#include "msm_drv.h"
+#ifdef CONFIG_DRM_MSM_KMS
+
#define MAX_PLANE 4
/* As there are different display controller blocks depending on the
@@ -196,4 +198,25 @@ void msm_drm_kms_post_init(struct device *dev);
void msm_drm_kms_unregister(struct device *dev);
void msm_drm_kms_uninit(struct device *dev);
+#else /* ! CONFIG_DRM_MSM_KMS */
+
+static inline int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
+{
+ return -ENODEV;
+}
+
+static inline void msm_drm_kms_post_init(struct device *dev)
+{
+}
+
+static inline void msm_drm_kms_unregister(struct device *dev)
+{
+}
+
+static inline void msm_drm_kms_uninit(struct device *dev)
+{
+}
+
+#endif
+
#endif /* __MSM_KMS_H__ */
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/7] drm/msm: make it possible to disable KMS-related code.
2025-04-13 16:32 ` [PATCH 3/7] drm/msm: make it possible to disable KMS-related code Dmitry Baryshkov
@ 2025-04-30 20:09 ` Abhinav Kumar
2025-05-02 10:09 ` Dmitry Baryshkov
0 siblings, 1 reply; 14+ messages in thread
From: Abhinav Kumar @ 2025-04-30 20:09 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 4/13/2025 9:32 AM, Dmitry Baryshkov wrote:
> If the Adreno device is used in a headless mode, there is no need to
> build all KMS components. Build corresponding parts conditionally, only
> selecting them if modeset support is actually required.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/Kconfig | 14 ++++++
> drivers/gpu/drm/msm/Makefile | 16 +++----
> drivers/gpu/drm/msm/dp/dp_debug.c | 4 ++
> drivers/gpu/drm/msm/msm_debugfs.c | 92 ++++++++++++++++++++++-----------------
> drivers/gpu/drm/msm/msm_drv.h | 7 ++-
> drivers/gpu/drm/msm/msm_kms.h | 23 ++++++++++
> 6 files changed, 108 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 974bc7c0ea761147d3326bdce9039d6f26f290d0..0be31c5acdd1b7ef80f0e820ff430692616122e0 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -66,6 +66,14 @@ config DRM_MSM_VALIDATE_XML
> Validate XML files with register definitions against rules-fd schema.
> This option is mostly targeting DRM MSM developers. If unsure, say N.
>
> +config DRM_MSM_KMS
> + def_bool n
> + depends on DRM_MSM
> +
> +config DRM_MSM_KMS_FBDEV
> + def_bool DRM_FBDEV_EMULATION
> + depends on DRM_MSM_KMS
> +
> config DRM_MSM_MDSS
> bool
> depends on DRM_MSM
> @@ -74,6 +82,7 @@ config DRM_MSM_MDSS
> config DRM_MSM_MDP4
> bool "Enable MDP4 support in MSM DRM driver"
> depends on DRM_MSM
> + select DRM_MSM_KMS
> default y
> help
> Compile in support for the Mobile Display Processor v4 (MDP4) in
> @@ -84,6 +93,7 @@ config DRM_MSM_MDP5
> bool "Enable MDP5 support in MSM DRM driver"
> depends on DRM_MSM
> select DRM_MSM_MDSS
> + select DRM_MSM_KMS
> default y
> help
> Compile in support for the Mobile Display Processor v5 (MDP5) in
> @@ -94,6 +104,7 @@ config DRM_MSM_DPU
> bool "Enable DPU support in MSM DRM driver"
> depends on DRM_MSM
> select DRM_MSM_MDSS
> + select DRM_MSM_KMS
> select DRM_DISPLAY_DSC_HELPER
> default y
> help
> @@ -104,6 +115,7 @@ config DRM_MSM_DPU
> config DRM_MSM_DP
> bool "Enable DisplayPort support in MSM DRM driver"
> depends on DRM_MSM
> + depends on DRM_MSM_KMS
> select RATIONAL
> default y
> help
> @@ -114,6 +126,7 @@ config DRM_MSM_DP
> config DRM_MSM_DSI
> bool "Enable DSI support in MSM DRM driver"
> depends on DRM_MSM
> + depends on DRM_MSM_KMS
> select DRM_PANEL
> select DRM_MIPI_DSI
> select DRM_DISPLAY_DSC_HELPER
> @@ -169,6 +182,7 @@ config DRM_MSM_DSI_7NM_PHY
> config DRM_MSM_HDMI
> bool "Enable HDMI support in MSM DRM driver"
> depends on DRM_MSM
> + depends on DRM_MSM_KMS
> default y
> select DRM_DISPLAY_HDMI_HELPER
> select DRM_DISPLAY_HDMI_STATE_HELPER
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 5df20cbeafb8bf07c825a1fd72719d5a56c38613..3749b0af779e73a93d7c219d3fbd3865b9296b50 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -101,18 +101,15 @@ msm-display-$(CONFIG_DRM_MSM_DPU) += \
> msm-display-$(CONFIG_DRM_MSM_MDSS) += \
> msm_mdss.o \
>
> -msm-display-y += \
> +msm-display-$(CONFIG_DRM_MSM_KMS) += \
> disp/mdp_format.o \
> disp/mdp_kms.o \
> disp/msm_disp_snapshot.o \
> disp/msm_disp_snapshot_util.o \
>
> msm-y += \
> - msm_atomic.o \
> - msm_atomic_tracepoints.o \
> msm_debugfs.o \
> msm_drv.o \
> - msm_fb.o \
> msm_fence.o \
> msm_gem.o \
> msm_gem_prime.o \
> @@ -123,21 +120,24 @@ msm-y += \
> msm_gpu_devfreq.o \
> msm_io_utils.o \
> msm_iommu.o \
> - msm_kms.o \
> msm_perf.o \
> msm_rd.o \
> msm_ringbuffer.o \
> msm_submitqueue.o \
> msm_gpu_tracepoints.o \
>
> -msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
> +msm-$(CONFIG_DRM_MSM_KMS) += \
> + msm_atomic.o \
> + msm_atomic_tracepoints.o \
> + msm_fb.o \
> + msm_kms.o \
>
> -msm-display-$(CONFIG_DEBUG_FS) += \
> - dp/dp_debug.o
> +msm-$(CONFIG_DRM_MSM_KMS_FBDEV) += msm_fbdev.o
>
> msm-display-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
> dp/dp_catalog.o \
> dp/dp_ctrl.o \
> + dp/dp_debug.o \
> dp/dp_display.o \
> dp/dp_drm.o \
> dp/dp_link.o \
> diff --git a/drivers/gpu/drm/msm/dp/dp_debug.c b/drivers/gpu/drm/msm/dp/dp_debug.c
> index 22fd946ee201397b72b43c8499714139deb7ba82..7835d93015b31a69aac824ca04dc65b374546e5c 100644
> --- a/drivers/gpu/drm/msm/dp/dp_debug.c
> +++ b/drivers/gpu/drm/msm/dp/dp_debug.c
> @@ -5,6 +5,8 @@
>
> #define pr_fmt(fmt)"[drm-dp] %s: " fmt, __func__
>
> +#ifdef CONFIG_DEBUG_FS
> +
> #include <linux/debugfs.h>
> #include <drm/drm_connector.h>
> #include <drm/drm_file.h>
> @@ -235,3 +237,5 @@ int msm_dp_debug_init(struct device *dev, struct msm_dp_panel *panel,
>
> return 0;
> }
> +
> +#endif
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
> index 7ab607252d183f78b99c3a8b878c949ed5f99fec..2b12f2851fadbc3c924827e11570352736869614 100644
> --- a/drivers/gpu/drm/msm/msm_debugfs.c
> +++ b/drivers/gpu/drm/msm/msm_debugfs.c
> @@ -117,6 +117,36 @@ static const struct file_operations msm_gpu_fops = {
> .release = msm_gpu_release,
> };
>
> +#ifdef CONFIG_DRM_MSM_KMS
> +static int msm_fb_show(struct seq_file *m, void *arg)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct drm_framebuffer *fb, *fbdev_fb = NULL;
> +
> + if (dev->fb_helper && dev->fb_helper->fb) {
> + seq_puts(m, "fbcon ");
> + fbdev_fb = dev->fb_helper->fb;
> + msm_framebuffer_describe(fbdev_fb, m);
> + }
> +
> + mutex_lock(&dev->mode_config.fb_lock);
> + list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
> + if (fb == fbdev_fb)
> + continue;
> +
> + seq_puts(m, "user ");
> + msm_framebuffer_describe(fb, m);
> + }
> + mutex_unlock(&dev->mode_config.fb_lock);
> +
> + return 0;
> +}
> +
> +static struct drm_info_list msm_kms_debugfs_list[] = {
> + { "fb", msm_fb_show },
> +};
> +
> /*
> * Display Snapshot:
> */
> @@ -180,6 +210,27 @@ static const struct file_operations msm_kms_fops = {
> .release = msm_kms_release,
> };
>
> +static void msm_debugfs_kms_init(struct drm_minor *minor)
> +{
> + struct drm_device *dev = minor->dev;
> + struct msm_drm_private *priv = dev->dev_private;
> +
> + drm_debugfs_create_files(msm_kms_debugfs_list,
> + ARRAY_SIZE(msm_kms_debugfs_list),
> + minor->debugfs_root, minor);
> + debugfs_create_file("kms", 0400, minor->debugfs_root,
> + dev, &msm_kms_fops);
> +
> + if (priv->kms->funcs->debugfs_init)
> + priv->kms->funcs->debugfs_init(priv->kms, minor);
> +
> +}
> +#else /* ! CONFIG_DRM_MSM_KMS */
> +static void msm_debugfs_kms_init(struct drm_minor *minor)
> +{
> +}
> +#endif
> +
> /*
> * Other debugfs:
> */
> @@ -238,40 +289,11 @@ static int msm_mm_show(struct seq_file *m, void *arg)
> return 0;
> }
>
> -static int msm_fb_show(struct seq_file *m, void *arg)
> -{
> - struct drm_info_node *node = m->private;
> - struct drm_device *dev = node->minor->dev;
> - struct drm_framebuffer *fb, *fbdev_fb = NULL;
> -
> - if (dev->fb_helper && dev->fb_helper->fb) {
> - seq_printf(m, "fbcon ");
> - fbdev_fb = dev->fb_helper->fb;
> - msm_framebuffer_describe(fbdev_fb, m);
> - }
> -
> - mutex_lock(&dev->mode_config.fb_lock);
> - list_for_each_entry(fb, &dev->mode_config.fb_list, head) {
> - if (fb == fbdev_fb)
> - continue;
> -
> - seq_printf(m, "user ");
> - msm_framebuffer_describe(fb, m);
> - }
> - mutex_unlock(&dev->mode_config.fb_lock);
> -
> - return 0;
> -}
> -
> static struct drm_info_list msm_debugfs_list[] = {
> {"gem", msm_gem_show},
> { "mm", msm_mm_show },
> };
>
> -static struct drm_info_list msm_kms_debugfs_list[] = {
> - { "fb", msm_fb_show },
> -};
> -
> static int late_init_minor(struct drm_minor *minor)
> {
> int ret;
> @@ -343,20 +365,12 @@ void msm_debugfs_init(struct drm_minor *minor)
> if (priv->gpu_pdev)
> msm_debugfs_gpu_init(minor);
>
> - if (priv->kms) {
> - drm_debugfs_create_files(msm_kms_debugfs_list,
> - ARRAY_SIZE(msm_kms_debugfs_list),
> - minor->debugfs_root, minor);
> - debugfs_create_file("kms", S_IRUSR, minor->debugfs_root,
> - dev, &msm_kms_fops);
> - }
> + if (priv->kms)
> + msm_debugfs_kms_init(minor);
>
> debugfs_create_file("shrink", S_IRWXU, minor->debugfs_root,
> dev, &shrink_fops);
>
> - if (priv->kms && priv->kms->funcs->debugfs_init)
> - priv->kms->funcs->debugfs_init(priv->kms, minor);
> -
> fault_create_debugfs_attr("fail_gem_alloc", minor->debugfs_root,
> &fail_gem_alloc);
> fault_create_debugfs_attr("fail_gem_iova", minor->debugfs_root,
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index a65077855201746c37ee742364b61116565f3794..5f4d3f050c1fde71c405a1ebf516f4f5a396cfc4 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -88,6 +88,7 @@ struct msm_drm_private {
> /* subordinate devices, if present: */
> struct platform_device *gpu_pdev;
>
> +#ifdef CONFIG_DRM_MSM_KMS
> /* possibly this should be in the kms component, but it is
> * shared by both mdp4 and mdp5..
> */
As the comment says, I am also thinking that this should be part of
msm_kms struct, to avoid ifdefs. I didnt follow the second half of the
comment that this is shared by both mdp4/mdp5. Why does that prevent it
from being in the kms component?
> @@ -97,6 +98,7 @@ struct msm_drm_private {
> struct msm_dsi *dsi[MSM_DSI_CONTROLLER_COUNT];
>
> struct msm_dp *dp[MSM_DP_CONTROLLER_COUNT];
> +#endif
>
> /* when we have more than one 'msm_gpu' these need to be an array: */
> struct msm_gpu *gpu;
> @@ -177,11 +179,13 @@ struct msm_drm_private {
> struct mutex lock;
> } lru;
>
> +#ifdef CONFIG_DRM_MSM_KMS
> struct workqueue_struct *wq;
>
> unsigned int num_crtcs;
>
> struct msm_drm_thread event_thread[MAX_CRTCS];
> +#endif
>
> /* VRAM carveout, used when no IOMMU: */
> struct {
> @@ -275,7 +279,7 @@ struct drm_framebuffer *msm_framebuffer_create(struct drm_device *dev,
> struct drm_framebuffer * msm_alloc_stolen_fb(struct drm_device *dev,
> int w, int h, int p, uint32_t format);
>
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> +#ifdef CONFIG_DRM_MSM_KMS_FBDEV
> int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
> struct drm_fb_helper_surface_size *sizes);
> #define MSM_FBDEV_DRIVER_OPS \
> @@ -360,6 +364,7 @@ static inline const char *msm_dsi_get_te_source(struct msm_dsi *msm_dsi)
> }
> #endif
>
> +struct msm_dp;
> #ifdef CONFIG_DRM_MSM_DP
> int __init msm_dp_register(void);
> void __exit msm_dp_unregister(void);
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 057b20367724a4ca60f2d60c038077dbcc1c7abc..aabbe851f0d533cca4a2ca00b79fbf8603870be3 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -13,6 +13,8 @@
>
> #include "msm_drv.h"
>
> +#ifdef CONFIG_DRM_MSM_KMS
> +
> #define MAX_PLANE 4
>
> /* As there are different display controller blocks depending on the
> @@ -196,4 +198,25 @@ void msm_drm_kms_post_init(struct device *dev);
> void msm_drm_kms_unregister(struct device *dev);
> void msm_drm_kms_uninit(struct device *dev);
>
> +#else /* ! CONFIG_DRM_MSM_KMS */
> +
> +static inline int msm_drm_kms_init(struct device *dev, const struct drm_driver *drv)
> +{
> + return -ENODEV;
> +}
> +
> +static inline void msm_drm_kms_post_init(struct device *dev)
> +{
> +}
> +
> +static inline void msm_drm_kms_unregister(struct device *dev)
> +{
> +}
> +
> +static inline void msm_drm_kms_uninit(struct device *dev)
> +{
> +}
> +
> +#endif
> +
> #endif /* __MSM_KMS_H__ */
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 3/7] drm/msm: make it possible to disable KMS-related code.
2025-04-30 20:09 ` Abhinav Kumar
@ 2025-05-02 10:09 ` Dmitry Baryshkov
0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-05-02 10:09 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Rob Clark, Dmitry Baryshkov, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Konrad Dybcio, linux-arm-msm,
dri-devel, freedreno, linux-kernel
On Wed, Apr 30, 2025 at 01:09:31PM -0700, Abhinav Kumar wrote:
>
>
> On 4/13/2025 9:32 AM, Dmitry Baryshkov wrote:
> > If the Adreno device is used in a headless mode, there is no need to
> > build all KMS components. Build corresponding parts conditionally, only
> > selecting them if modeset support is actually required.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > ---
> > drivers/gpu/drm/msm/Kconfig | 14 ++++++
> > drivers/gpu/drm/msm/Makefile | 16 +++----
> > drivers/gpu/drm/msm/dp/dp_debug.c | 4 ++
> > drivers/gpu/drm/msm/msm_debugfs.c | 92 ++++++++++++++++++++++-----------------
> > drivers/gpu/drm/msm/msm_drv.h | 7 ++-
> > drivers/gpu/drm/msm/msm_kms.h | 23 ++++++++++
> > 6 files changed, 108 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> > index a65077855201746c37ee742364b61116565f3794..5f4d3f050c1fde71c405a1ebf516f4f5a396cfc4 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.h
> > +++ b/drivers/gpu/drm/msm/msm_drv.h
> > @@ -88,6 +88,7 @@ struct msm_drm_private {
> > /* subordinate devices, if present: */
> > struct platform_device *gpu_pdev;
> > +#ifdef CONFIG_DRM_MSM_KMS
> > /* possibly this should be in the kms component, but it is
> > * shared by both mdp4 and mdp5..
> > */
>
> As the comment says, I am also thinking that this should be part of msm_kms
> struct, to avoid ifdefs. I didnt follow the second half of the comment that
> this is shared by both mdp4/mdp5. Why does that prevent it from being in the
> kms component?
Indeed, there are no such limitations nowadays.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/7] drm/msm: bail out late_init_minor() if it is not a GPU device
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
` (2 preceding siblings ...)
2025-04-13 16:32 ` [PATCH 3/7] drm/msm: make it possible to disable KMS-related code Dmitry Baryshkov
@ 2025-04-13 16:32 ` Dmitry Baryshkov
2025-04-13 16:32 ` [PATCH 5/7] drm/msm: rearrange symbol selection Dmitry Baryshkov
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:32 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Both perf and hangrd make sense only for GPU devices. Bail out if we are
registering a KMS-only device.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_debugfs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index 2b12f2851fadbc3c924827e11570352736869614..6e60a74b13d72c47e45cb9dc65ed67b977e900fa 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -296,11 +296,16 @@ static struct drm_info_list msm_debugfs_list[] = {
static int late_init_minor(struct drm_minor *minor)
{
+ struct drm_device *dev = minor->dev;
+ struct msm_drm_private *priv = dev->dev_private;
int ret;
if (!minor)
return 0;
+ if (!priv->gpu_pdev)
+ return 0;
+
ret = msm_rd_debugfs_init(minor);
if (ret) {
DRM_DEV_ERROR(minor->dev->dev, "could not install rd debugfs\n");
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/7] drm/msm: rearrange symbol selection
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
` (3 preceding siblings ...)
2025-04-13 16:32 ` [PATCH 4/7] drm/msm: bail out late_init_minor() if it is not a GPU device Dmitry Baryshkov
@ 2025-04-13 16:32 ` Dmitry Baryshkov
2025-04-13 16:33 ` [PATCH 6/7] drm/msm: rework binding of Imageon GPUs Dmitry Baryshkov
2025-04-13 16:33 ` [PATCH 7/7] drm/msm: enable separate binding of GPU and display devices Dmitry Baryshkov
6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:32 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Move symbol selection to be more fine grained: select DP helpers only if
DP driver is also enabled, move KMS and display helpers to the newly
introduced DRM_MSM_KMS.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/Kconfig | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 0be31c5acdd1b7ef80f0e820ff430692616122e0..b3c6117ab035764d603cc257c28df651d9c38175 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -15,18 +15,8 @@ config DRM_MSM
select IOMMU_IO_PGTABLE
select QCOM_MDT_LOADER if ARCH_QCOM
select REGULATOR
- select DRM_CLIENT_SELECTION
- select DRM_DISPLAY_DP_AUX_BUS
- select DRM_DISPLAY_DP_HELPER
- select DRM_DISPLAY_HELPER
- select DRM_BRIDGE_CONNECTOR
select DRM_EXEC
- select DRM_KMS_HELPER
- select DRM_PANEL
- select DRM_BRIDGE
- select DRM_PANEL_BRIDGE
select DRM_SCHED
- select FB_SYSMEM_HELPERS if DRM_FBDEV_EMULATION
select SHMEM
select TMPFS
select QCOM_SCM
@@ -69,10 +59,18 @@ config DRM_MSM_VALIDATE_XML
config DRM_MSM_KMS
def_bool n
depends on DRM_MSM
+ select DRM_BRIDGE
+ select DRM_BRIDGE_CONNECTOR
+ select DRM_CLIENT_SELECTION
+ select DRM_DISPLAY_HELPER
+ select DRM_KMS_HELPER
+ select DRM_PANEL
+ select DRM_PANEL_BRIDGE
config DRM_MSM_KMS_FBDEV
def_bool DRM_FBDEV_EMULATION
depends on DRM_MSM_KMS
+ select FB_SYSMEM_HELPERS
config DRM_MSM_MDSS
bool
@@ -117,6 +115,8 @@ config DRM_MSM_DP
depends on DRM_MSM
depends on DRM_MSM_KMS
select RATIONAL
+ select DRM_DISPLAY_DP_AUX_BUS
+ select DRM_DISPLAY_DP_HELPER
default y
help
Compile in support for DP driver in MSM DRM driver. DP external
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/7] drm/msm: rework binding of Imageon GPUs
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
` (4 preceding siblings ...)
2025-04-13 16:32 ` [PATCH 5/7] drm/msm: rearrange symbol selection Dmitry Baryshkov
@ 2025-04-13 16:33 ` Dmitry Baryshkov
2025-04-13 16:33 ` [PATCH 7/7] drm/msm: enable separate binding of GPU and display devices Dmitry Baryshkov
6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:33 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Currently the msm driver creates an extra interim platform device for
Imageon GPUs. This is not ideal, as the device doesn't have
corresponding OF node. If the headless mode is used for newer GPUs, then
the msm_use_mmu() function can not detect corresponding IOMMU devices.
Also the DRM device (although it's headless) is created with modesetting
flags being set.
To solve all these issues, rework the way the Imageon devices are bound.
Remove the interim device, don't register a component and instead use a
cut-down version of the normal functions to probe or remove the driver.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 36 +++---------
drivers/gpu/drm/msm/msm_drv.c | 91 ++++++++++++++++++++----------
drivers/gpu/drm/msm/msm_drv.h | 4 ++
3 files changed, 72 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 236b25c094cd5d462f4b6653de7b7910985cccb6..325cb710ea08ac8e5c3d9c80c8d8e18e1946e994 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -241,42 +241,22 @@ static const struct component_ops a3xx_ops = {
.unbind = adreno_unbind,
};
-static void adreno_device_register_headless(void)
-{
- /* on imx5, we don't have a top-level mdp/dpu node
- * this creates a dummy node for the driver for that case
- */
- struct platform_device_info dummy_info = {
- .parent = NULL,
- .name = "msm",
- .id = -1,
- .res = NULL,
- .num_res = 0,
- .data = NULL,
- .size_data = 0,
- .dma_mask = ~0,
- };
- platform_device_register_full(&dummy_info);
-}
-
static int adreno_probe(struct platform_device *pdev)
{
-
- int ret;
-
- ret = component_add(&pdev->dev, &a3xx_ops);
- if (ret)
- return ret;
-
if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon"))
- adreno_device_register_headless();
+ return msm_gpu_probe(pdev, &a3xx_ops);
- return 0;
+ return component_add(&pdev->dev, &a3xx_ops);
}
static void adreno_remove(struct platform_device *pdev)
{
- component_del(&pdev->dev, &a3xx_ops);
+ struct msm_drm_private *priv = platform_get_drvdata(pdev);
+
+ if (priv->kms_init)
+ component_del(&pdev->dev, &a3xx_ops);
+ else
+ msm_gpu_remove(pdev, &a3xx_ops);
}
static void adreno_shutdown(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b296d1315aa249638e073d08d43e8d41a4354f3b..e5c1124d45fa146c91caaad39a39fb9d21e5f5f3 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -62,7 +62,7 @@ module_param(modeset, bool, 0600);
DECLARE_FAULT_ATTR(fail_gem_alloc);
DECLARE_FAULT_ATTR(fail_gem_iova);
-static int msm_drm_uninit(struct device *dev)
+static int msm_drm_uninit(struct device *dev, const struct component_ops *gpu_ops)
{
struct platform_device *pdev = to_platform_device(dev);
struct msm_drm_private *priv = platform_get_drvdata(pdev);
@@ -91,7 +91,10 @@ static int msm_drm_uninit(struct device *dev)
msm_deinit_vram(ddev);
- component_unbind_all(dev, ddev);
+ if (gpu_ops)
+ gpu_ops->unbind(dev, dev, NULL);
+ else
+ component_unbind_all(dev, ddev);
ddev->dev_private = NULL;
drm_dev_put(ddev);
@@ -200,7 +203,8 @@ static void msm_deinit_vram(struct drm_device *ddev)
attrs);
}
-static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
+static int msm_drm_init(struct device *dev, const struct drm_driver *drv,
+ const struct component_ops *gpu_ops)
{
struct msm_drm_private *priv = dev_get_drvdata(dev);
struct drm_device *ddev;
@@ -247,7 +251,10 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
dma_set_max_seg_size(dev, UINT_MAX);
/* Bind all our sub-components: */
- ret = component_bind_all(dev, ddev);
+ if (gpu_ops)
+ ret = gpu_ops->bind(dev, dev, NULL);
+ else
+ ret = component_bind_all(dev, ddev);
if (ret)
goto err_deinit_vram;
@@ -259,11 +266,6 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
ret = msm_drm_kms_init(dev, drv);
if (ret)
goto err_msm_uninit;
- } else {
- /* valid only for the dummy headless case, where of_node=NULL */
- WARN_ON(dev->of_node);
- ddev->driver_features &= ~DRIVER_MODESET;
- ddev->driver_features &= ~DRIVER_ATOMIC;
}
ret = drm_dev_register(ddev, 0);
@@ -280,7 +282,7 @@ static int msm_drm_init(struct device *dev, const struct drm_driver *drv)
return 0;
err_msm_uninit:
- msm_drm_uninit(dev);
+ msm_drm_uninit(dev, gpu_ops);
return ret;
@@ -896,6 +898,28 @@ static const struct drm_driver msm_driver = {
.patchlevel = MSM_VERSION_PATCHLEVEL,
};
+static const struct drm_driver msm_gpu_driver = {
+ .driver_features = DRIVER_GEM |
+ DRIVER_RENDER |
+ DRIVER_SYNCOBJ_TIMELINE |
+ DRIVER_SYNCOBJ,
+ .open = msm_open,
+ .postclose = msm_postclose,
+ .gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
+#ifdef CONFIG_DEBUG_FS
+ .debugfs_init = msm_debugfs_init,
+#endif
+ .show_fdinfo = msm_show_fdinfo,
+ .ioctls = msm_ioctls,
+ .num_ioctls = ARRAY_SIZE(msm_ioctls),
+ .fops = &fops,
+ .name = "msm",
+ .desc = "MSM Snapdragon DRM",
+ .major = MSM_VERSION_MAJOR,
+ .minor = MSM_VERSION_MINOR,
+ .patchlevel = MSM_VERSION_PATCHLEVEL,
+};
+
/*
* Componentized driver support:
*/
@@ -1020,12 +1044,12 @@ static int add_gpu_components(struct device *dev,
static int msm_drm_bind(struct device *dev)
{
- return msm_drm_init(dev, &msm_driver);
+ return msm_drm_init(dev, &msm_driver, NULL);
}
static void msm_drm_unbind(struct device *dev)
{
- msm_drm_uninit(dev);
+ msm_drm_uninit(dev, NULL);
}
const struct component_master_ops msm_drm_ops = {
@@ -1074,29 +1098,34 @@ int msm_drv_probe(struct device *master_dev,
return 0;
}
-/*
- * Platform driver:
- * Used only for headlesss GPU instances
- */
-
-static int msm_pdev_probe(struct platform_device *pdev)
+int msm_gpu_probe(struct platform_device *pdev,
+ const struct component_ops *ops)
{
- return msm_drv_probe(&pdev->dev, NULL, NULL);
+ struct msm_drm_private *priv;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+
+ /* on all devices that I am aware of, iommu's which can map
+ * any address the cpu can see are used:
+ */
+ ret = dma_set_mask_and_coherent(&pdev->dev, ~0);
+ if (ret)
+ return ret;
+
+ return msm_drm_init(&pdev->dev, &msm_gpu_driver, ops);
}
-static void msm_pdev_remove(struct platform_device *pdev)
+void msm_gpu_remove(struct platform_device *pdev,
+ const struct component_ops *ops)
{
- component_master_del(&pdev->dev, &msm_drm_ops);
+ msm_drm_uninit(&pdev->dev, ops);
}
-static struct platform_driver msm_platform_driver = {
- .probe = msm_pdev_probe,
- .remove = msm_pdev_remove,
- .driver = {
- .name = "msm",
- },
-};
-
static int __init msm_drm_register(void)
{
if (!modeset)
@@ -1111,13 +1140,13 @@ static int __init msm_drm_register(void)
adreno_register();
msm_mdp4_register();
msm_mdss_register();
- return platform_driver_register(&msm_platform_driver);
+
+ return 0;
}
static void __exit msm_drm_unregister(void)
{
DBG("fini");
- platform_driver_unregister(&msm_platform_driver);
msm_mdss_unregister();
msm_mdp4_unregister();
msm_dp_unregister();
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 5f4d3f050c1fde71c405a1ebf516f4f5a396cfc4..78c4f0ca689cc93b8dba3fae061a92923e3115a5 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -558,6 +558,10 @@ extern const struct component_master_ops msm_drm_ops;
int msm_kms_pm_prepare(struct device *dev);
void msm_kms_pm_complete(struct device *dev);
+int msm_gpu_probe(struct platform_device *pdev,
+ const struct component_ops *ops);
+void msm_gpu_remove(struct platform_device *pdev,
+ const struct component_ops *ops);
int msm_drv_probe(struct device *dev,
int (*kms_init)(struct drm_device *dev),
struct msm_kms *kms);
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 7/7] drm/msm: enable separate binding of GPU and display devices
2025-04-13 16:32 [PATCH 0/7] drm/msm: rework the ties between KMS and GPU parts of the driver Dmitry Baryshkov
` (5 preceding siblings ...)
2025-04-13 16:33 ` [PATCH 6/7] drm/msm: rework binding of Imageon GPUs Dmitry Baryshkov
@ 2025-04-13 16:33 ` Dmitry Baryshkov
6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Baryshkov @ 2025-04-13 16:33 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Konrad Dybcio
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
There are cases when we want to have separate DRM devices for GPU and
display pipelines.
One example is development, when it is beneficial to be able to bind the
GPU driver separately, without the display pipeline (and without the
hacks adding "amd,imageon" to the compatible string).
Another example is some of Qualcomm platforms, which have two MDSS
units, but only one GPU. With current approach it is next to impossible
to support this usecase properly, while separate binding allows users to
have three DRM devices: two for MDSS units and a single headless GPU.
Add kernel param msm.separate_gpu_drm, which if set to true forces
creation of separate display and GPU DRM devices. Mesa supports this
setup by using the kmsro wrapper.
The param is disabled by default, in order to be able to test userspace
for the compatibility issues. Simple clients are able to handle this
setup automatically.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 3 +-
drivers/gpu/drm/msm/msm_drv.c | 49 +++++++++++++++++++++++++++---
drivers/gpu/drm/msm/msm_drv.h | 2 ++
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 325cb710ea08ac8e5c3d9c80c8d8e18e1946e994..2322a3301a5226c4e2590344e4744934addeea33 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -243,7 +243,8 @@ 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"))
+ if (of_device_is_compatible(pdev->dev.of_node, "amd,imageon") ||
+ msm_gpu_no_components())
return msm_gpu_probe(pdev, &a3xx_ops);
return component_add(&pdev->dev, &a3xx_ops);
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e5c1124d45fa146c91caaad39a39fb9d21e5f5f3..4502425d0a62e1efaca5b987fa5b657dc0a4e927 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -59,9 +59,18 @@ 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_drm;
+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_drm, bool, 0400);
+
DECLARE_FAULT_ATTR(fail_gem_alloc);
DECLARE_FAULT_ATTR(fail_gem_iova);
+bool msm_gpu_no_components(void)
+{
+ return separate_gpu_drm;
+}
+
static int msm_drm_uninit(struct device *dev, const struct component_ops *gpu_ops)
{
struct platform_device *pdev = to_platform_device(dev);
@@ -898,6 +907,32 @@ static const struct drm_driver msm_driver = {
.patchlevel = MSM_VERSION_PATCHLEVEL,
};
+static const struct drm_driver msm_kms_driver = {
+ .driver_features = DRIVER_GEM |
+ DRIVER_ATOMIC |
+ DRIVER_MODESET |
+ DRIVER_SYNCOBJ_TIMELINE |
+ DRIVER_SYNCOBJ,
+ .open = msm_open,
+ .postclose = msm_postclose,
+ .dumb_create = msm_gem_dumb_create,
+ .dumb_map_offset = msm_gem_dumb_map_offset,
+ .gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
+#ifdef CONFIG_DEBUG_FS
+ .debugfs_init = msm_debugfs_init,
+#endif
+ MSM_FBDEV_DRIVER_OPS,
+ .show_fdinfo = msm_show_fdinfo,
+ .ioctls = msm_ioctls,
+ .num_ioctls = ARRAY_SIZE(msm_ioctls),
+ .fops = &fops,
+ .name = "msm-kms",
+ .desc = "MSM Snapdragon DRM",
+ .major = MSM_VERSION_MAJOR,
+ .minor = MSM_VERSION_MINOR,
+ .patchlevel = MSM_VERSION_PATCHLEVEL,
+};
+
static const struct drm_driver msm_gpu_driver = {
.driver_features = DRIVER_GEM |
DRIVER_RENDER |
@@ -1044,7 +1079,11 @@ static int add_gpu_components(struct device *dev,
static int msm_drm_bind(struct device *dev)
{
- return msm_drm_init(dev, &msm_driver, NULL);
+ return msm_drm_init(dev,
+ msm_gpu_no_components() ?
+ &msm_kms_driver :
+ &msm_driver,
+ NULL);
}
static void msm_drm_unbind(struct device *dev)
@@ -1080,9 +1119,11 @@ int msm_drv_probe(struct device *master_dev,
return ret;
}
- ret = add_gpu_components(master_dev, &match);
- if (ret)
- return ret;
+ if (!msm_gpu_no_components()) {
+ ret = add_gpu_components(master_dev, &match);
+ if (ret)
+ return ret;
+ }
/* on all devices that I am aware of, iommu's which can map
* any address the cpu can see are used:
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 78c4f0ca689cc93b8dba3fae061a92923e3115a5..73dd90cf4f17ae1b9e29d4309eed74393a99d6db 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -569,4 +569,6 @@ 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);
+
#endif /* __MSM_DRV_H__ */
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread