[PATCH 0/2] drm/msm: Split gpu/kms cleanups

[PATCH 0/2] drm/msm: Split gpu/kms cleanups

[Rob Clark]

Applies on top of https://lore.kernel.org/all/20250705-msm-gpu-split-v4-0-fb470c481131@oss.qualcomm.com/

Rob Clark (2):
  drm/msm: Clean up split driver features
  drm/msm: Take the ioctls away from the KMS-only driver

 drivers/gpu/drm/msm/msm_drv.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

-- 
2.50.0

[PATCH 1/2] drm/msm: Clean up split driver features

[Rob Clark]

Avoid the possibility of missing features between the split and unified
drm driver cases by defining DRIVER_FEATURES_GPU / KMS and using those
in the drm_driver initializations.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_drv.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index b6efc5b9933b..5695de1bbae2 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -816,14 +816,21 @@ static const struct file_operations fops = {
 	.show_fdinfo = drm_show_fdinfo,
 };
 
+#define DRIVER_FEATURES_GPU ( \
+		DRIVER_GEM | \
+		DRIVER_GEM_GPUVA | \
+		DRIVER_RENDER | \
+		DRIVER_SYNCOBJ_TIMELINE | \
+		0 )
+
+#define DRIVER_FEATURES_KMS ( \
+		DRIVER_GEM | \
+		DRIVER_ATOMIC | \
+		DRIVER_MODESET | \
+		0 )
+
 static const struct drm_driver msm_driver = {
-	.driver_features    = DRIVER_GEM |
-				DRIVER_GEM_GPUVA |
-				DRIVER_RENDER |
-				DRIVER_ATOMIC |
-				DRIVER_MODESET |
-				DRIVER_SYNCOBJ_TIMELINE |
-				DRIVER_SYNCOBJ,
+	.driver_features    = DRIVER_FEATURES_GPU | DRIVER_FEATURES_KMS,
 	.open               = msm_open,
 	.postclose          = msm_postclose,
 	.dumb_create        = msm_gem_dumb_create,
@@ -846,9 +853,7 @@ static const struct drm_driver msm_driver = {
 };
 
 static const struct drm_driver msm_kms_driver = {
-	.driver_features    = DRIVER_GEM |
-				DRIVER_ATOMIC |
-				DRIVER_MODESET,
+	.driver_features    = DRIVER_FEATURES_KMS,
 	.open               = msm_open,
 	.postclose          = msm_postclose,
 	.dumb_create        = msm_gem_dumb_create,
@@ -870,11 +875,7 @@ static const struct drm_driver msm_kms_driver = {
 };
 
 static const struct drm_driver msm_gpu_driver = {
-	.driver_features    = DRIVER_GEM |
-				DRIVER_GEM_GPUVA |
-				DRIVER_RENDER |
-				DRIVER_SYNCOBJ_TIMELINE |
-				DRIVER_SYNCOBJ,
+	.driver_features    = DRIVER_FEATURES_GPU,
 	.open               = msm_open,
 	.postclose          = msm_postclose,
 	.gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
-- 
2.50.0

Re: [PATCH 1/2] drm/msm: Clean up split driver features

[Dmitry Baryshkov]

On Sat, Jul 05, 2025 at 07:52:40AM -0700, Rob Clark wrote:
> Avoid the possibility of missing features between the split and unified
> drm driver cases by defining DRIVER_FEATURES_GPU / KMS and using those
> in the drm_driver initializations.
> 
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_drv.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH 1/2] drm/msm: Clean up split driver features

[Konrad Dybcio]

On 7/5/25 4:52 PM, Rob Clark wrote:
> Avoid the possibility of missing features between the split and unified
> drm driver cases by defining DRIVER_FEATURES_GPU / KMS and using those
> in the drm_driver initializations.
> 
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_drv.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index b6efc5b9933b..5695de1bbae2 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -816,14 +816,21 @@ static const struct file_operations fops = {
>  	.show_fdinfo = drm_show_fdinfo,
>  };
>  
> +#define DRIVER_FEATURES_GPU ( \
> +		DRIVER_GEM | \
> +		DRIVER_GEM_GPUVA | \
> +		DRIVER_RENDER | \
> +		DRIVER_SYNCOBJ_TIMELINE | \
> +		0 )
> +
> +#define DRIVER_FEATURES_KMS ( \
> +		DRIVER_GEM | \
> +		DRIVER_ATOMIC | \
> +		DRIVER_MODESET | \
> +		0 )

Perhaps I'm missing some C lore, but do we need the "| 0"?

Konrad

Re: [PATCH 1/2] drm/msm: Clean up split driver features

[Rob Clark]

On Mon, Jul 7, 2025 at 4:42 AM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 7/5/25 4:52 PM, Rob Clark wrote:
> > Avoid the possibility of missing features between the split and unified
> > drm driver cases by defining DRIVER_FEATURES_GPU / KMS and using those
> > in the drm_driver initializations.
> >
> > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/msm_drv.c | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index b6efc5b9933b..5695de1bbae2 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -816,14 +816,21 @@ static const struct file_operations fops = {
> >       .show_fdinfo = drm_show_fdinfo,
> >  };
> >
> > +#define DRIVER_FEATURES_GPU ( \
> > +             DRIVER_GEM | \
> > +             DRIVER_GEM_GPUVA | \
> > +             DRIVER_RENDER | \
> > +             DRIVER_SYNCOBJ_TIMELINE | \
> > +             0 )
> > +
> > +#define DRIVER_FEATURES_KMS ( \
> > +             DRIVER_GEM | \
> > +             DRIVER_ATOMIC | \
> > +             DRIVER_MODESET | \
> > +             0 )
>
> Perhaps I'm missing some C lore, but do we need the "| 0"?

It is just to avoid later diffstat noise when new flags are added :-)

BR,
-R

[PATCH 2/2] drm/msm: Take the ioctls away from the KMS-only driver

[Rob Clark]

KMS-only drivers should only allocate dumb buffers.  The driver custom
ioctls are only meant for the usermode gpu driver (mesa), and not for
general consumption, so they don't make sense for standalone KMS
drivers.

Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/msm_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 5695de1bbae2..3c856c0ae855 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -864,8 +864,6 @@ static const struct drm_driver msm_kms_driver = {
 #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",
-- 
2.50.0

Re: [PATCH 2/2] drm/msm: Take the ioctls away from the KMS-only driver

[Dmitry Baryshkov]

On Sat, Jul 05, 2025 at 07:52:41AM -0700, Rob Clark wrote:
> KMS-only drivers should only allocate dumb buffers.  The driver custom
> ioctls are only meant for the usermode gpu driver (mesa), and not for
> general consumption, so they don't make sense for standalone KMS
> drivers.
> 
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/msm_drv.c | 2 --
>  1 file changed, 2 deletions(-)
> 

If we need GEM IOCTLs for the KMS node, we can think about it later.
Let's remove them for now.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry